On Apr 14, 2009, at 13:03, Sanne Grinovero wrote:
I've 2 inline answers:
2009/4/14 Emmanuel Bernard <emman...@hibernate.org>:
Read inline
On Apr 13, 2009, at 17:22, Sanne Grinovero wrote:
After our chat about the topic I thought that I only needed some
minor
changes,
was quite wrong.
I moved the flush listener to the usual
FullTextIndexEventListener, using
delegation at first as we had agreed. This got me into troubles with
the Serialization test of the FullTextEntityManager, I had to adapt
EventSourceTransactionContext changing a field to transient and
having the code manage the case in which the values are lost by
deserialization.
After this I removed the delegation moving the code to the
FullTextIndexEventListener
which resulted in simpler code.
You made me think about what was happening in case of an error
during
the flush processing in the default listener, so I've replaced the
Map
with a combination
of non-static ThreadLocal with a weak reference to the flushing
Session,
which is checked for == identity to make sure the synch is
relevant to
the same session,
in case of a previously not cleaned-up flush by the same Thread
(as in
a Map, but I only need
the last stored value for the current session, and only for the same
thread).
I think I don't like the ThreadLocal approach. What is your
reasoning for
using a thread local variable? What would make it less compelling
if we were
not using a TL?
the two event listeners "on-post-somethingchanged" and "flush" are
executed
one after the other right away, atomically from a out-of-hibernate-
search point
of view. In the test code if I put breakpoints in both the "register
synch" routing
and "find the synch from the flushlistener" thay both happen only when
the client code is asking to flush.
This made me think the two routines are always being called by the
same thread,
and there is no chance to begin doing something else between the
first operation
and the second operation: it's not possible that, for example, the
Session is
passivated or suspendend (thinking about long running conversations).
The only way to stop the second to happen is having
an exception in the default hibernate flush listener: that's the
reason for the weak
reference to Session.
These thoughts leaded me to think that a concurrent map is overkill,
especially
considering that every thread in the application is to access
this map twice per flush: to put in an object, and get it back
immediately after that,
and no thread trying to say anything to the others or wanting to store
it for a longer
time. So I thought the ThreadLocal was the natural answer.
Still the resulting code is ugly, I'm happy to forget my performance
thoughts if we
could get with a cleaner solution.
I've another version ready using a WeakIdentityHashMap, it's also
passing the tests,
and is a bit more readable, but I'm unsure about which is better.
Shall I commit the one using WeakIdentityHashMap instead of
ThreadLocal?
I looked that the TL impl in Java SE and they are hosted on a per
Thread object basis, so I guess there is no concurrency lock here. So
it all boils down to which is faster:
- a concurrent map look up access
- getting the current Thread object + a map lookup (internal TL
variable impl)
Up to you. I have the tendency to limit my TL use as it has caused
issues in the past.
Remember this part of the code is used only as a workaround for people
misconfiguring HSearch so I am not too fussed about the perf.
_______________________________________________
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev
