On Aug 8, 2013, at 5:45 AM, A. Sundararajan
<[email protected]> wrote:
> I'd like to lazily initialize InvokeByName and dynamic method handles in
> global instance. I am not sure of the refactoring in NativeArray that you
> suggested.
>
> Are you talking about these?
>
> private final MethodHandle someInvoker =
> getSOME_CALLBACK_INVOKER();
>
> If I pass key to a refactored method, I've to do if..else on object identity
> check.. Or am I missing something?
This is what I had in mind:
private static MethodHandle getEVERY_CALLBACK_INVOKER() {
return createIteratorCallbackInvoker(EVERY_CALLBACK_INVOKER, boolean.class);
}
private static MethodHandle getSOME_CALLBACK_INVOKER() {
return createIteratorCallbackInvoker(SOME_CALLBACK_INVOKER, boolean.class);
}
...
private static createIteratorCallbackInvoker(final Object key, final Class<?>
rtype) {
return Global.instance().getDynamicInvoker(key,
new Callable<MethodHandle>() {
@Override
public MethodHandle call() {
return createIteratorCallbackInvoker(rtype);
}
});
}
>
> Also, on avoiding synchronized in Global.java. If we'd like to avoid jdk8
> specific API/syntax in nashorn, I can't use computeIfAbsent. But putIfAbsent
> forces computing the value... Again, am I missing something?
private final ConcurrentMap<Object, InvokeByName> namedInvokers = new
ConcurrentHashMap<>();
@Override
public InvokeByName getInvokeByName(final Object key, final
Callable<InvokeByName> creator) {
final InvokeByName invoke = namedInvokers.get(key);
if(invoke != null) {
return invoke;
}
final InvokeByName newInvoke = creator.call();
final InvokeByName existingInvoke = namedInvokers.putIfAbsent(key,
newInvoke);
return existingInvoke != null ? existingInvoke : newInvoke;
}
>
> thanks,
> -Sundar
>
> On Thursday 08 August 2013 08:56 AM, A. Sundararajan wrote:
>> On Thursday 08 August 2013 02:29 AM, Attila Szegedi wrote:
>>> - CompileUnit: While making fields non-final and nulling out fields is
>>> certainly a solution, I don't like it as it feels fragile - you end up with
>>> an object that has a member nulled out, and what if something later would
>>> want to depend on it etc. As an example, consider CompileUnit, which now
>>> has its ClassEmitter nulled out. Seems like architecturally it's a better
>>> idea is to remove the field from the CompileUnit altogether, and use a
>>> composite object being a tuple of (CompileUnit, ClassEmitter) in the
>>> compiler, and only pass down the CompileUnit part of the tuple to things in
>>> the IR package that require it.
>>
>> While code can be refactored for a longer term, as of now, it does leak
>> memory. Moment class is loaded, we don't need lots of info maintained by
>> ASM's ClassEmitter. I suggest we go with short term solution and revisit
>> refactoring changes to FunctionNode/CompileUnit/Compiler later.
>>
>>> - Another issue I have is with synchronization in the Global object; I'd
>>> rather use a ConcurrentMap and the (new for Java 8) computeIfAbsent()
>>> method.
>>> <http://download.java.net/jdk8/docs/api/java/util/Map.html#computeIfAbsent(K,
>>> java.util.function.Function)>. If you don't want to rely on
>>> computeIfAbsent() (but I don't see why wouldn't you, frankly), you could
>>> still use a composition of get() and putIfAbsent().
>>
>> We still don't use any jdk8 specific API in nashorn codebase yet (I
>> believe). I'll restructure this with older API.
>>> - In NativeArray, you could factor out the pattern of getting an invoker
>>> for an iterator callback repeated across 4 methods into a method taking a
>>> key and a return type.
>> Will do.
>>
>>> - Ostensibly, NativeObject could just use Global.TO_STRING instead of
>>> having its own now. Not too convinced about this, as these things sort-of
>>> represent call sites, so maybe it's okay as it is.
>>
>> Yes - it is a different callsite (although I doubt how much InvokeByName and
>> dynamic invokers help now!)
>>> - We still keep GlobalObject interface around?
>>
>> Yes - we do. That calls for more refactorings. As I said, I'd like to keep
>> it minimal (as much as possible) for now.
>>>
>>> - Why does RecompilableScriptFunctionData.ensureHasAllocator have to be
>>> synchronized? If we absolutely need atomic updates to the allocator field,
>>> I'd consider using an AtomicReference for it instead. Having
>>> synchronization in path of every "new SomeClass()" bothers me. Even if it's
>>> completely unsynced and the field is not volatile, we only "risk" creating
>>> the method handle multiple times; shouldn't be a big deal as we're (a)
>>> rarely multithreaded and (b) it's idempotent. So, I'd rather choose a bit
>>> of a statistical redundancy than a certain performance hit.
>>>
>>> - Why does ensureCodeGenerated have to be synchronized? Can the
>>> modifications of fields possibly occur on multiple threads? I mean,
>>> functionNode.canSpecialize() will be determined at first execution and
>>> fields nulled out. Also, wouldn't a second call to ensureCodeGenerated()
>>> after functionNode was nulled out (if that's possible) result in a NPE on
>>> functionNode.isLazy(), or is this guarded by !code.isEmpty()? At least this
>>> synchronization only happens once on every linking event and not on every
>>> invocation, unlike allocate() but I still don't really see the necessity.
>>
>> I'll check again.
>>
>> -Sundar
>>
>>>
>>> Attila.
>>>
>>> On Aug 7, 2013, at 6:56 PM, A. Sundararajan
>>> <[email protected]> wrote:
>>>
>>>> Please review http://cr.openjdk.java.net/~sundar/8022524/
>>>>
>>>> Thanks
>>>> -Sundar
>>
>
