[hibernate-dev] HSEARCH-189 & concurrency
Emmanuel Bernard
emmanuel at hibernate.org
Fri Jul 4 16:48:29 EDT 2008
Doh I need System.Threading.Thread.MemoryBarrier() but in Java :(
--
Emmanuel Bernard
http://in.relation.to/Bloggers/Emmanuel | http://blog.emmanuelbernard.com
| http://twitter.com/emmanuelbernard
Hibernate Search in Action (http://is.gd/Dl1)
On Jul 4, 2008, at 16:26, Emmanuel Bernard wrote:
> Hey,
> Can you tell me more about the piggyback synchronization. I could
> not find any decent knowledge online.
> how far reading a volatile guarantee that all "local" values of the
> thread we are reading from will be pushed to the shared memory?
> For example, could reading a volatile value after HSearch is done
> with initialization (all init is done in a single thread) guarantee
> that all states held by this thead will be pushed back to the shared
> memory?
> The use case is quite specific, I init everything in a single
> thread, want to push all the state to the shared memory. I know post
> init() use of HSearch will never change the state so I don't "need"
> locking.
>
> --
> Emmanuel Bernard
> http://in.relation.to/Bloggers/Emmanuel | http://blog.emmanuelbernard.com
> | http://twitter.com/emmanuelbernard
> Hibernate Search in Action (http://is.gd/Dl1)
>
> On Jul 4, 2008, at 04:33, Sanne Grinovero wrote:
>
>> Hi Pavitar Singh,
>>
>> I thank you very much about your explanations but actually I opened
>> the issue myself
>> because I have read the same specs and am aware of that.
>> in H.Search (and several other hibernate code) there's this quite
>> common pattern for starting
>> "replaceable" objects (something like user-written plugins, you can
>> provide your own implementation
>> to do some stuff) but this same pattern is also used to start the
>> built-in default strategies.
>>
>> It looks like this:
>> - an empty constructor, to use class.newInstance();
>> - an initialize() to set configuration options
>> - a start() method (eventually) used to start background tasks
>> - some doStuff() and/or getXX() which need to be fast & threadsafe
>>
>> As you can see in Concurrecy in Practice at page 50, this is BAD,
>> as for
>> example in the FSSlaveDirectoryProvider nobody takes care of locking
>> or visibility, and nobody is doing anywhere where I see this
>> pattern used
>> (several times in the project).
>> I'm not saying it is all broken, because usually the threads
>> consuming
>> these unsafely-initialized objects are started after the
>> initialization, so
>> that's ok. In this specific case the state will be used to
>> communicate
>> between threads, so some visibility fix is needed.
>>
>> I know I could use it only for final fields, but this is exactly
>> what I want:
>> there are currently 10 instance variables, of these
>> 4 have no concurrent use
>> 4 are configuration constants and could use the "final" (they're
>> not safely published)
>> 2 would need some lock/volatile anyway, bot only one is used
>> frequently, so IMHO 1 volatile is ok.
>>
>> I was thinking in using the same Piggyback technique you mention to
>> safely publish the initialization constants,
>> but I'm afraid the code will become more difficult to maintain and
>> more
>> "unreadable", possibly breaking at the first next patch:
>> IMHO using some unchanging fields "final" is the most clean and
>> readable solution (and best performing), but I need a different
>> constructor for that.
>>
>> your opinion is really welcome as I couldn't find other feedback,
>> if you would like to take a look at the sources download the Search
>> trunk
>> and look at:
>> org.hibernate.search.store.FSSlaveDirectoryProvider
>> or the FIXME in
>> org.hibernate.search.reader.SharingBufferReaderProvider
>>
>> Sanne
>>
>> 2008/7/4 Pavitar Singh <pavitar.singh at gmail.com>:
>> Hi Sanne,
>>
>> I don't think moving everything in constructor can guarantee safe
>> publication.
>>
>> From the JMM Specification Section 3.5
>>
>> "An object is considered to be completely initialized when its
>> constructor finishes. A thread that can only see a reference to an
>> object after that object has been completely initialized is
>> guaranteed to see the correctly initialized values for that
>> object's final fields."
>>
>> That means there are no guarantees for other fields which are non-
>> final.
>>
>> But once things are moved in constructor then by using Safe
>> Publication one can enforce visibility guarantees.
>>
>> Other thing i was wondering was why do one need to make every field
>> as volatile. As just by using a single volatile variable one can
>> enforce memory barriers. A technique documented in JCIP as
>> Piggyback Synchronizations and used by Doug Lea in implementing
>> ConcurrentHashMap.(You will find get method in ConcurrentHashMap is
>> without any locking and visibility is enforced by use of a single
>> volatile variable.)
>>
>> Also if you can elaborate more on how you are facing the visibility
>> issue then may be i can also spend time on it on figuring
>> performant solution.
>>
>> Regards
>> Pavitar Singh
>> http://pitfalls.wordpress.com
>>
>> On Fri, Jul 4, 2008 at 5:13 AM, Sanne Grinovero <sanne.grinovero at gmail.com
>> > wrote:
>> Hello all,
>> I'm sorry I've been very busy but as promised I would like to fix
>> HSEARCH-189
>> (and others) very soon;
>>
>> I would like to propose a API extension (backwards compatible) that
>> would simplify the patch a lot:
>> keeping it as is it is very tricky to fix the visibility issues in
>> FSSlaveDirectoryProvider
>> and FSMasterDirectoryProvider without introducing a performance
>> penalty.
>>
>> I have these options to close the issue:
>> 1) add a "volatile" to more than six fields per class (ugly and not
>> good for performance)
>> 2) use some Locks/synch (more readable, still performance hits)
>> 3) move the "initialize" arguments to a constructor.
>>
>> As Emmanuel knows I would really love the third option, but he's
>> worried about
>> the fact we can't force a constructor in an interface*1, so my
>> proposal is:
>>
>> if we find there exists a constructor having the same arguments as
>> the initialize method,
>> we use that, otherwise we use a no-arg constructor and then call
>> the initialize.
>>
>> Reflection is used anyway to instantiate these components,
>> the code in DirectoryProviderFactory doesn't get much more
>> complicated
>> and much more cleanup is made possible in all DPs because of this
>> (as the equals/hashcode comments already ask for).
>>
>> I actually think this same pattern is needed for other components,
>> such as all ReaderProvider, so I hope you'll want to give it a try
>> and let me apply it on other components too if you like the
>> resulting code.
>>
>> Sanne
>>
>> _______________________________________________
>> hibernate-dev mailing list
>> hibernate-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>
>>
>>
>>
>> --
>> Pavitar Singh
>> Blog: http://pitfalls.wordpress.com
>>
>> _______________________________________________
>> hibernate-dev mailing list
>> hibernate-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>
> _______________________________________________
> hibernate-dev mailing list
> hibernate-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/hibernate-dev/attachments/20080704/b35a3a6c/attachment.html
More information about the hibernate-dev
mailing list