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.
