On 25 Sep 2014, at 11:50, Attila Szegedi <[email protected]> wrote:
> MemberInfo.java: s/thre/the
> BufferArray.java: s/Constructur/Constructor
>
> ApplySpecialization: Set<Expression> argumentsFound could be a boolean
> instead of a set.
>
I need a writable datastructure that is still final outside the loop. I’ll
leave this for now. Might just as easily have done final boolean result[] = new
boolean[1] or something… also ugly.
> I asked for comments in Global.extractBuiltinProperties; I don't see them
> having been added.
>
Added
> Not directly part of this (but rather earlier work leading up to this), but I
> think Global.BOOTSTRAP is a very misleading name. Also, the whole mechanism
> to invokedynamic through INVALIDATE_NAME_BOOTSTRAP seems too roundabout to
> me. Seems to me we could do a straight invokestatic to
> Global.invalidateReservedName().
>
Done. All that logic gone. See below.
> BTW: after thinking about this I noticed that Global.invalidateReservedName()
> is an instance method, not a static method. I can see you bind
> INVALIDATE_RESERVED_NAME to a Global. Two remarks about that: (1) instance
> fields shouldn't be named to look like static constants (2) This all gives
> the mistaken impression that invalidation is still per-global instead of
> per-context. Really, the whole invokedynamic through
> Global.invalidateNameBootstrap should be replaced with an INVOKESTATIC to a
> method in ScriptRuntime (which is meant to be our primary script-to-runtime
> API), and Global should have no traces of this logic; it's not the place for
> it.
>
Fixed. All the logic is moved to a static call in ScriptRuntime, as it should
be. Thanks for spotting this. This is actually legacy from my first
implementation, before I fixed this for all builtin properties.
> That phrase "because one of the thinks" in a comment to NativeArray.
> setIsLengthNotWritable makes no sense; needs some proofreading.
> NativeArray.popInt() JavaDoc has some stray " + " markers in it. Method's
> exceptions should be declared in a @throws, not @return
> NativeArray.pushInt() JavaDoc: @throws vs. @return too; some others too
>
Fixed.
> Does LinkLogic have to have a cache for the Switchpoints? I'm asking because
> if it didn't, then you also wouldn't have to have two fields for
> PushLinkLogic and PopLinkLogic in NativeArray, as you wouldn't have to cache
> those either but could just always create new instances in getLinkLogic. Just
> a thought – I can accept that you can have genuine additional reasons to
> cache switchpoints in LinkLogic.
>
No we don’t need that cache performance wise, but I currently don’t want to
create new link logic instances per callsite I link for push and pop. I am
reluctant to change this now and have to redo all performance measurements so
I’d rather leave it for later, if I may. I don’t think it matters, but I don’t
want to cause any last minute problems.
> FinalScriptFunctionData: why are addInvoker methods now returning
> CompiledFunction instead of void? Doesn't seem to be used.
>
Left it like that too, I used them for constructing invoker lists in the
beginning, but not right now, but I see no harm in exposing the return invokers
as return values, it makes a more flexible API possible where you can chain
addInvoker calls for example
> GlobalFunctions: admittedly a matter of taste/style, but all those constant
> and identity method handles; I might've just declared them as small private
> static Java methods instead, e.g. public static int parseInt(Object this, int
> i) { return i; } instead of playing with combinators. Works either way, just
> we end up with more lambdaforms…
>
I left it like it is, because it is consitent to the rest of the
specializations right now.
> ScriptObject.isProto() seems unused. In any case should be named hasProto…
>
Removed
> Why not remove ScriptObject.checkReservedName?
Removed
>
> That's all.
> Attila.
>
> On Sep 24, 2014, at 2:31 PM, Marcus Lagergren <[email protected]>
> wrote:
>
>> OK, new webrev addressing Attila’s comments and Sundar’s and Hannes’s
>> offline comments as well is up at :
>> http://cr.openjdk.java.net/~lagergren/8025435.2/
>>
>> I’ve now moved, as suggested, all abstraction about various optimistic
>> strategies into the native objects themselves, which indeed cleaned stuff up
>> nicely. The result is shorter overall codemass. The problem with primitives
>> and optimistic builtins was present in the old branch itself, only it did
>> not manifest itself.
>>
>> Test is clean. Leaving to pick up the kids now, and will run test262 and
>> octane when I am home with the goal of checking that everything is correct
>> and verifying my octane performance improvements on a dedicated benchmarking
>> machine.
>>
>> Regards
>> Marcus
>>
>>
>> On 23 Sep 2014, at 17:36, Marcus Lagergren <[email protected]>
>> wrote:
>>
>>> Guys, guys! This is my review thread for JDK-8025435. Start another one for
>>> this, please…
>>>
>>> /M
>>>
>>> On 23 Sep 2014, at 17:29, Aleksey Shipilev <[email protected]>
>>> wrote:
>>>
>>>> On 09/23/2014 05:58 PM, Thomas Wuerthinger wrote:
>>>>> I understood your remark. I just believe that this restriction is not
>>>>> fundamental to JMH and there *could* be support for languages that do
>>>>> not statically compile to Java bytecodes as well.
>>>>
>>>> I think this thread side-tracks.
>>>>
>>>> Bottom-line: There are only two ways to hook up dynamically-compiled
>>>> language runtime into JMH (say, JS engine), depending who is in control:
>>>>
>>>> a) Ask JS engine to start executing the JS benchmark, *and* call back
>>>> into JMH with an AST/bytecode/whatever in hands, asking to measure it.
>>>> We would need to generate the synthetic Java code, compile it, load it,
>>>> and only then run it, calling back to JS runtime for execution of our
>>>> payload -- all in flight, and requiring the tight cohesion.
>>>>
>>>> b) Ask JMH to compile and execute a Java benchmark, and call into JS
>>>> engine with a JS script in hands, asking to compile and run it. There,
>>>> users statically compile the benchmark JAR, and it runs with minimal
>>>> dynamic dances, also going through the standard APIs (javax.script).
>>>>
>>>> While there is nothing fundamental preventing us from exploring the
>>>> route (a), it is not as practical as going the well-established route
>>>> (b). In other words, "could be done" != "should be done".
>>>>
>>>> -Aleksey.
>>>>
>>>
>>
>
