On 05/16/2014 01:56 PM, Tobias Hartmann wrote:
Hi,
thanks to everyone for the feedback.
Is it sufficient then to use synchronized (lambdaForms) { ... } in
setCachedLambdaForm(..) and a normal read in cachedLambdaForm(..)?
Thanks,
Tobias
no, it's not,
The memory model doesn't guarantee that a normal read will see that
another thread has changed the array item,
the normal read may always see null.
You need a synchronized block around the normal read.
Rémi
I may be wrong but for me cachedLambdaForm() is used in a fast
path.
If it's not the case, I agree that a lock is enough.
I hold the opinion that we only need to synchronize writers and not
readers (MTF.setCachedLambdaForm() and not MTF.cachedLambdaForm()).
Current usage pattern is the following:
MethodType type = ...;
LambdaForm lform = type.form().cachedLambdaForm(idx);
if (lform == NULL) {
// construct new LambaForm
lform = type.form().setCachedLambdaForm(idx, lform);
}
return lform;
Cache is written only once, so it has only 2 states (null and
non-null
value) during it's lifecycle.
The only stale value cachedLambdaForm() can see is null, but
then the
caller tries to initialize the cache and after acquiring the
lock in
setCachedLambdaForm() sees actual value.
So, the worst thing can happen if readers aren't synchronized is
creation of unnecessary LF (which go dead right away) in rare
cases.
Unless the cache is volatile this would be unsafe publication and
the
reader may see a non-null reference to the LF but the LF may not be
properly initialized.
Good point. The cache is an array element. How does safe publication
of array element look like? Is accessing the element through volatile
reference to the array enough or additional synchronization actions
are needed?
I thought that since the only fields explicitly set when constructing
a LambdaForm are final fields it should be safe to publish the form
reference as an element in the array?
Paul.
that's my analysis too.
Sorry didn't realize a LF only had final fields.
David
Rémi
