RFR: 8142334: Improve lazy initialization of java.lang.invoke
Peter Levart
peter.levart at gmail.com
Sun Nov 15 09:29:28 UTC 2015
Hi Vladimir,
On 11/14/2015 12:59 AM, Vladimir Ivanov wrote:
> NamedFunction.resolvedHandle should be usually pre-resolved, but for
> bootstrapping purposes it is done lazily in some cases. I don't see
> any reason why NamedFunction.resolve() should be called on a freshly
> created instance. Use proper constructor instead.
I agree, but I think that's just a style issue. Even if you use a proper
constructor, the fact that resolvedHandle is not a final field means
that such instance has to be published safely to other threads or you
have to guarantee that resolvedHandle of such instance is accessed only
through accessor method that checks for null and (re)initializes the
field if it finds it still being null.
In the later case (when resolvedHandle is pre-populated and
NamedFunction is published unsafely), it can happen that some threads
get the pre-populated instance of resolvedHandle and other threads that
don't yet see the pre-populated instance (because of reorderings) get
the lazily resolved instance computed by one of them (if double-checked
locking is used).
So if always returning a single instance from resolvedHandle() accessor
is important, resolvedHandle field should not be pre-populated if unsafe
publication of NamedFunction instance is exercised. What role does
pre-resolving play? I only see it as a means to throw exceptions early.
So perhaps an assert in selected constructors that resolves the handle
but does not assign it to resolvedHandle field could catch any coding
mistakes in tests for the price of double-resolving when assertions are
enabled.
> NamedFunction(MethodHandle resolvedHandle)
> NamedFunction(MemberName member, MethodHandle resolvedHandle)
> NamedFunction(MethodType basicInvokerType)
>
> // The next 3 constructors are used to break circular
> dependencies on MH.invokeStatic, etc.
> // Any LambdaForm containing such a member is not interpretable.
> // This is OK, since all such LFs are prepared with special
> primitive vmentry points.
> // And even without the resolvedHandle, the name can still be
> compiled and optimized.
> NamedFunction(Method method) {
> this(new MemberName(method));
> }
> NamedFunction(Field field) {
> this(new MemberName(field));
> }
> NamedFunction(MemberName member) {
> this.member = member;
> this.resolvedHandle = null;
> }
>
> There are 2 non-final fields in NamedFunction:
> static class NamedFunction {
> final MemberName member;
> @Stable MethodHandle resolvedHandle;
> @Stable MethodHandle invoker;
>
> Repeated initialization of NamedFunction.invoker is benign since
> NamedFunction.computeInvoker uses synchronized
> MethodTypeForm.setCachedMethodHandle to populate the cache.
>
> Regarding resolvedHandle, NamedFunction are usually constructed
> pre-resolved, but for the limited number of lazily initialized
> instances, I think we should force resolution right after
> bootstrapping is over.
But then they are not lazily initialized any more. Or do you refer to
eagerly initialized instances that are not pre-resolved?
> Or, if you want to save on resolvedHandle resolution, replace all
> direct field accesses with accessor calls and synchronize when setting
> the value.
I think there can only be two kinds of "safe" NamedFunctions:
- pre-resolved and published safely (these are the ones that are
initialized in <clinit> of a class and published via a static field of
the same class)
- lazily initialized, not-yet-resolved and which may be published unsafely
The refactoring that Claes is making is changind the 1st kind of
NamedFunction(s) into the 2nd kind.
Only the 1st kind of NamedFunction(s) can be used so that their
resolvedHandle field is directly accessed in code.
Because it is difficult to track through code which ones are which and
it's easy to make a mistake that is hard to track down, it would be best
to always access resolvedHandle via it's accessor anyway, to make code
more robust to changes such as Claes'.
What do you think?
Regards, Peter
>
>
> Best regards,
> Vladimir Ivanov
>
> On 11/12/15 10:11 PM, Claes Redestad wrote:
>>
>> On 2015-11-12 19:53, Peter Levart wrote:
>>>>
>>>> Do you think this would work correctly w.r.t visibility:
>>>>
>>>> FUNCTIONS[idx] = function;
>>>> function.resolve();
>>>> UNSAFE.storeFence();
>>>
>>> This does not do anything useful.
>>>
>>> What happens if you simply omit function.resolve() call. If lazy
>>> resolving works and tests pass, then perhaps it's all ok.
>>>
>>> Peter
>>
>> Actually, I simply tried backing out the DirectMethodHandle changes
>> entirely, and measured how much we'd lose by leaving that particular
>> case alone: almost nothing, as it turns out (same number of LFs are
>> generated etc). I think the safest option is to leave DirectMethodHandle
>> alone:
>>
>> http://cr.openjdk.java.net/~redestad/8142334/webrev.05/
>>
>> /Claes
More information about the core-libs-dev
mailing list