Re: Changes in Java 9

[Nils Kilden-Pedersen]

Excellent, that appears to work.

Thanks both!

On Fri, Dec 15, 2017 at 7:19 AM, Sundararajan Athijegannathan <
sundararajan.athijegannat...@oracle.com> wrote:

> Adding to what Hannes said. In case if you cannot modify existing code,
> you can also pass options to nashorn engine via System property
> "nashorn.args" - all nashorn engines created in the process will be
> initialized with those options.
>
> For example:
>
>java -Dnashorn.args="-ot=false" Main
>
> See also: https://wiki.openjdk.java.net/display/Nashorn/Nashorn+jsr223
> +engine+notes
>
> -Sundar
>
>
> On 15/12/17, 6:10 PM, Hannes Wallnöfer wrote:
>
>> Hi Nils,
>>
>> Thanks for the code. Unfortunately you hit a bug in optimistic types,
>> which is enabled by default in JDK 9. I’ve filed an issue for it and have a
>> fix for it.
>>
>> https://bugs.openjdk.java.net/browse/JDK-8193508
>>
>> The good news is that it is quite easy to work around this bug by
>> disabling optimistic types. The way to do this is to use the
>> NashornScriptEngineFactory class to create the script engine:
>>
>> import jdk.nashorn.api.scripting.NashornScriptEngineFactory;
>>
>> ...
>>  ScriptEngine engine = new NashornScriptEngineFactory().g
>> etScriptEngine("-ot=false");
>>
>> Hannes
>>
>>
>> Am 06.12.2017 um 22:30 schrieb Nils Kilden-Pedersen:
>>>
>>> Hannes,
>>>
>>> I've sent an email with zip file attachment.
>>>
>>> Just letting you know in a separate email, in case it ends up being
>>> blocked.
>>>
>>> Nils
>>>
>>> On Wed, Dec 6, 2017 at 2:19 PM, Hannes Wallnöfer>> acle.com>  wrote:
>>> That would be https://bugs.openjdk.java.net/ but you need an OpenJDK
>>> account to file a bug.
>>>
>>> Alternatively, if you can send me something reproduce the bug (could be
>>> a small snippet of code) I can file it for you.
>>>
>>> Hannes
>>>
>>>
>>> Am 06.12.2017 um 16:15 schrieb Nils Kilden-Pedersen:

 What's the right place to file a bug?

 On Mon, Dec 4, 2017 at 9:56 AM, Nils Kilden-Pedersen
 wrote:

 Thanks for bringing this up. It made me realize that my post was
> incomplete.
>
> These are the steps I take:
>
>1. Initial step: Call engine.compile with the CS compiler source +
>CoffeeScript.compile(coffeeCode, {runtime: 'none'}); which returns
>CompiledScript that is retained and re-used. This works fine, also
> on
>JDK 9.0.1.
>2. For subsequent Coffeescript ->  Javascript transpilation, I then
>call engine.createBindings()
>3. Then bindings.put("coffeeCode", csCode) to associate the CS code
>with the JS variable coffeeCode mentioned in initial step
>4. Call CompiledScript.eval(bindings). This is the step that fails
> in
>9.0.1.
>
> I’m open to better ways to achieve this, if it’s inefficient or plain
> wrong.
> ​
>
> On Mon, Dec 4, 2017 at 8:47 AM, Hannes Wallnöfer<
> hannes.wallnoe...@oracle.com>  wrote:
>
> Hi Nils,
>>
>> Are you just evaluating the script files you linked in your first
>> message, or trying to do some further processing?
>>
>> Using the jjs tool from JDK 9.0.1 I see no errors running both
>> versions
>> of CoffeeScript.
>>
>> Hannes
>>
>> Am 03.12.2017 um 21:08 schrieb Nils Kilden-Pedersen>> >:
>>>
>>> Ok, the minified vs non-minified may not be identical. I cannot find
>>> the
>>> default parameters in the minified version, so perhaps those are
>>>
>> removed.
>>
>>> What does remain however, is that I can compile minified cs2
>>>   on
>>> JDK
>>>
>> 8_144,
>>
>>> but not on 9.0.1.
>>>
>>>
>>> On Sun, Dec 3, 2017 at 1:39 PM, Nils Kilden-Pedersen>> om>
>>> wrote:
>>>
>>> Further testing with the un-minified code (cs2
 ,

>>> cs1
>>
>>> )
 reveals the true problems in JDK 9.0.1

 In cs2, it’s ES6 syntax (default args), which (unexpectedly for me
 at
 least), works in JDK 8:

:46:45 Expected , but found =
 CoffeeScript.eval = function(code, options = {}) {
 ^ in  at line
 number

>>> 46 at column number 45
>>
>>> And in cs1 it’s require that’s unresolved (also works in JDK 8):

 ReferenceError: "require" is not defined in  at line number 7

 ​

 On Sun, Dec 3, 2017 at 11:02 AM, Nils Kilden-Pedersen<

>>> nil...@gmail.com>
>>
>>> wrote:

 I just switched to the Java 9.0.1 runtime, 

Re: Changes in Java 9

[Hannes Wallnöfer]

Hi Nils,

Thanks for the code. Unfortunately you hit a bug in optimistic types, which is 
enabled by default in JDK 9. I’ve filed an issue for it and have a fix for it.

https://bugs.openjdk.java.net/browse/JDK-8193508

The good news is that it is quite easy to work around this bug by disabling 
optimistic types. The way to do this is to use the NashornScriptEngineFactory 
class to create the script engine:

import jdk.nashorn.api.scripting.NashornScriptEngineFactory; 

...
ScriptEngine engine = new 
NashornScriptEngineFactory().getScriptEngine("-ot=false"); 

Hannes


> Am 06.12.2017 um 22:30 schrieb Nils Kilden-Pedersen :
> 
> Hannes,
> 
> I've sent an email with zip file attachment. 
> 
> Just letting you know in a separate email, in case it ends up being blocked.
> 
> Nils
> 
> On Wed, Dec 6, 2017 at 2:19 PM, Hannes Wallnöfer 
>  wrote:
> That would be https://bugs.openjdk.java.net/ but you need an OpenJDK account 
> to file a bug.
> 
> Alternatively, if you can send me something reproduce the bug (could be a 
> small snippet of code) I can file it for you.
> 
> Hannes
> 
> 
> > Am 06.12.2017 um 16:15 schrieb Nils Kilden-Pedersen :
> >
> > What's the right place to file a bug?
> >
> > On Mon, Dec 4, 2017 at 9:56 AM, Nils Kilden-Pedersen 
> > wrote:
> >
> >> Thanks for bringing this up. It made me realize that my post was
> >> incomplete.
> >>
> >> These are the steps I take:
> >>
> >>   1. Initial step: Call engine.compile with the CS compiler source +
> >>   CoffeeScript.compile(coffeeCode, {runtime: 'none'}); which returns
> >>   CompiledScript that is retained and re-used. This works fine, also on
> >>   JDK 9.0.1.
> >>   2. For subsequent Coffeescript -> Javascript transpilation, I then
> >>   call engine.createBindings()
> >>   3. Then bindings.put("coffeeCode", csCode) to associate the CS code
> >>   with the JS variable coffeeCode mentioned in initial step
> >>   4. Call CompiledScript.eval(bindings). This is the step that fails in
> >>   9.0.1.
> >>
> >> I’m open to better ways to achieve this, if it’s inefficient or plain
> >> wrong.
> >> ​
> >>
> >> On Mon, Dec 4, 2017 at 8:47 AM, Hannes Wallnöfer <
> >> hannes.wallnoe...@oracle.com> wrote:
> >>
> >>> Hi Nils,
> >>>
> >>> Are you just evaluating the script files you linked in your first
> >>> message, or trying to do some further processing?
> >>>
> >>> Using the jjs tool from JDK 9.0.1 I see no errors running both versions
> >>> of CoffeeScript.
> >>>
> >>> Hannes
> >>>
>  Am 03.12.2017 um 21:08 schrieb Nils Kilden-Pedersen :
> 
>  Ok, the minified vs non-minified may not be identical. I cannot find the
>  default parameters in the minified version, so perhaps those are
> >>> removed.
> 
>  What does remain however, is that I can compile minified cs2
>   on JDK
> >>> 8_144,
>  but not on 9.0.1.
> 
> 
>  On Sun, Dec 3, 2017 at 1:39 PM, Nils Kilden-Pedersen 
>  wrote:
> 
> > Further testing with the un-minified code (cs2
> > ,
> >>> cs1
> > )
> > reveals the true problems in JDK 9.0.1
> >
> > In cs2, it’s ES6 syntax (default args), which (unexpectedly for me at
> > least), works in JDK 8:
> >
> >   :46:45 Expected , but found =
> > CoffeeScript.eval = function(code, options = {}) {
> >^ in  at line number
> >>> 46 at column number 45
> >
> > And in cs1 it’s require that’s unresolved (also works in JDK 8):
> >
> > ReferenceError: "require" is not defined in  at line number 7
> >
> > ​
> >
> > On Sun, Dec 3, 2017 at 11:02 AM, Nils Kilden-Pedersen <
> >>> nil...@gmail.com>
> > wrote:
> >
> >> I just switched to the Java 9.0.1 runtime, and Nashorn now fails to
> >>> parse
> >> the Coffeescript compiler, which worked fine in Java 8_144.
> >>
> >> Coffeescript 2 compiler: http://coffeescript.org/v2/bro
> >> wser-compiler/coffeescript.js
> >>
> >> v1 also fails: http://coffeescript.org/v1/bro
> >> wser-compiler/coffee-script.js
> >>
> >> Exception is:
> >>
> >> javax.script.ScriptException: SyntaxError: unexpected  in  at
> >>> line number 39 at column number 249736
> >> at jdk.scripting.nashorn/jdk.nashorn.api.scripting.NashornScrip
> >>> tEngine.throwAsScriptException(NashornScriptEngine.java:469)
> >> at jdk.scripting.nashorn/jdk.nashorn.api.scripting.NashornScrip
> >>> tEngine.evalImpl(NashornScriptEngine.java:425)
> >> at jdk.scripting.nashorn/jdk.nashorn.api.scripting.NashornScrip
> >>> tEngine.access$300(NashornScriptEngine.java:72)
> >> at 

Re: Review request for JDK-8193371: Use Dynalink REMOVE operation in Nashorn

[Hannes Wallnöfer]

Nice work indeed.

+1

Hannes

> Am 14.12.2017 um 12:42 schrieb Attila Szegedi :
> 
> Please review JDK-8193371 "Use Dynalink REMOVE operation in Nashorn" at 
>  for 
> 
> 
> It looks bigger than it is. let me quickly explain this change in detail:
> 
> There are some things that could make sense even if we didn’t refit delete 
> operator on Dynalink REMOVE.
> 
> * Fist such thing is moving the evaluation logic for the delete operator from 
> AssignSymbols into CodeGenerator. That way, we eliminated a need to carry 
> information from AS to CG in a RuntimeNode, and we could retire all of 
> DELETE, SLOW_DELETE, and FAIL_DELETE RuntimeNode enums. It also allowed us to 
> have cleaner expression of the code generator, e.g strict flag is handled 
> more elegantly when deleting an IdentNode, and also we can just emit 
> invocation of ScriptObject.delete on the scope object for IdentNode removals. 
> There’s overall stronger typing (e.g. slow delete implementation has a better 
> signature).
> 
> * By not erasing “delete” UnaryNodes in AssignSymbols anymore, I had to take 
> a bit of special care in LocalVariableTypesCalculator to not treat deletes of 
> IdentNodes as uses.
> 
> * The changes in control flow of AbstractJavaLinker and BeanLinker 
> getGuardedInvocationComponent are actual bug fixes that became apparent to me 
> when I realized while writing the tests that the code as it was was failing 
> to link REMOVE:PROPERTY|ELEMENT; it was incorrectly not trying all the 
> namespaces it should’ve. (The bug was more generic than just that one 
> operation combination: it’d affect e.g. SET:METHOD|PROPERTY too and some 
> other combinations).
> 
> As for actual delete operator through Dynalink:
> 
> * MethodEmitter gained methods for emitting dynamic call sites for removal. 
> Those are very similar to existing ones for dynamic getters and setters, 
> nothing special there.
> 
> * The logic formerly in ScriptRuntime.DELETE has now been spread into 
> relevant linkers: ScriptObject.lookup, JSObjectLinker, Undefined.lookup, and 
> NashornBottomLinker. The case in ScriptRuntime.DELETE for “if 
> (JSType.isPrimitive(obj))” doesn’t even have to be handled separately, 
> primitive linking that instantiates wrappers takes care nicely of it.
> 
> * That said, ScriptRuntime.DELETE had a final “return true” fallback for 
> POJOs, and I decided to improve on it. For one thing, theres an automatic 
> improvement because delete now works as expected on Java lists and maps just 
> by virtue of Dynalink providing that functionality. Another improvement that 
> I decided to introduce manually is found in the way NashornBottomLinker 
> handles delete on POJOs, specifically the uses of isNonConfigurableProperty. 
> I tried treating all properties and methods of POJOs as if they were 
> non-configurable properties (we try to treat them like that elsewhere in the 
> code as well), so attempts to delete them will return false (non-strict) or 
> throw a TypeError (strict), in accordance with ES 8.12.7 [[Delete]]. The 
> newly added test file tries to exercise all new behavior.
> 
> * Finally, NashornCallSiteDescriptor now needs 4 bits for the operation as we 
> went from 8 operations to 10 with “delete obj.name” and “delete obj[index]". 
> This again halved the number of program points available to 131072, but 
> that's fortunately still about 10x what is really needed, so we should be 
> good.
> 
> Thanks,
>  Attila.
> 

Re: Review request for JDK-8193371: Use Dynalink REMOVE operation in Nashorn

[Sundararajan Athijegannathan]

+1

Nice work!

-Sundar

On 14/12/17, 5:12 PM, Attila Szegedi wrote:

Please review JDK-8193371 "Use Dynalink REMOVE operation in Nashorn" 
at  
for

It looks bigger than it is. let me quickly explain this change in detail:

There are some things that could make sense even if we didn’t refit delete 
operator on Dynalink REMOVE.

* Fist such thing is moving the evaluation logic for the delete operator from 
AssignSymbols into CodeGenerator. That way, we eliminated a need to carry 
information from AS to CG in a RuntimeNode, and we could retire all of DELETE, 
SLOW_DELETE, and FAIL_DELETE RuntimeNode enums. It also allowed us to have 
cleaner expression of the code generator, e.g strict flag is handled more 
elegantly when deleting an IdentNode, and also we can just emit invocation of 
ScriptObject.delete on the scope object for IdentNode removals. There’s overall 
stronger typing (e.g. slow delete implementation has a better signature).

* By not erasing “delete” UnaryNodes in AssignSymbols anymore, I had to take a 
bit of special care in LocalVariableTypesCalculator to not treat deletes of 
IdentNodes as uses.

* The changes in control flow of AbstractJavaLinker and BeanLinker 
getGuardedInvocationComponent are actual bug fixes that became apparent to me 
when I realized while writing the tests that the code as it was was failing to 
link REMOVE:PROPERTY|ELEMENT; it was incorrectly not trying all the namespaces 
it should’ve. (The bug was more generic than just that one operation 
combination: it’d affect e.g. SET:METHOD|PROPERTY too and some other 
combinations).

As for actual delete operator through Dynalink:

* MethodEmitter gained methods for emitting dynamic call sites for removal. 
Those are very similar to existing ones for dynamic getters and setters, 
nothing special there.

* The logic formerly in ScriptRuntime.DELETE has now been spread into relevant 
linkers: ScriptObject.lookup, JSObjectLinker, Undefined.lookup, and 
NashornBottomLinker. The case in ScriptRuntime.DELETE for “if 
(JSType.isPrimitive(obj))” doesn’t even have to be handled separately, 
primitive linking that instantiates wrappers takes care nicely of it.

* That said, ScriptRuntime.DELETE had a final “return true” fallback for POJOs, 
and I decided to improve on it. For one thing, theres an automatic improvement 
because delete now works as expected on Java lists and maps just by virtue of 
Dynalink providing that functionality. Another improvement that I decided to 
introduce manually is found in the way NashornBottomLinker handles delete on 
POJOs, specifically the uses of isNonConfigurableProperty. I tried treating all 
properties and methods of POJOs as if they were non-configurable properties (we 
try to treat them like that elsewhere in the code as well), so attempts to 
delete them will return false (non-strict) or throw a TypeError (strict), in 
accordance with ES 8.12.7 [[Delete]]. The newly added test file tries to 
exercise all new behavior.

* Finally, NashornCallSiteDescriptor now needs 4 bits for the operation as we went 
from 8 operations to 10 with “delete obj.name” and “delete obj[index]". This 
again halved the number of program points available to 131072, but that's 
fortunately still about 10x what is really needed, so we should be good.

Thanks,
   Attila.