SA: Various fixes to sa.js to make it work in JDK8

A. Sundararajan sundararajan.athijegannathan at oracle.com
Tue Jul 30 00:50:39 PDT 2013


Hi Kris,

All the JavaScript changes look good to me. Thanks for taking care of 
rhino-vs-nashorn compatibility issues.

PS. I used to work in hotspot serviceability agent in the past. Now, I'm 
with the nashorn team.

Thanks
-Sundar

On Monday 29 July 2013 01:23 PM, Krystal Mok wrote:
> Hi all,
>
> Could I have a couple of reviews for this patch, please?
>
> https://gist.github.com/rednaxelafx/6102608
>
> Switching the JavaScript engine from Rhino to Nashorn and the No 
> PermGen project caused a few issues that stopped sa.js from working. 
> This is a patch that tries to fix the issues that I've run into.
>
> I'll walkthrough the patch below: (line numbers refer to sajs.patch, 
> not sa.js)
>
> Line 8-12:
>
> Since Nashorn implements ECMAScript 5.1, using JavaScript keywords as 
> the property name in the "dot" syntax is not a problem anymore. So I'm 
> un-commenting these lines.
>
> Line 20-42, 51-76:
>
> When implementing a JavaAdapter for SA ScriptObject, the __get__ 
> function calls the __has__ function:
>   this.__has__(name)
> which is working at the wrong level: __has__ is a special hook 
> function, and cannot be called via "this" this way. It'll trigger the 
> __call__ hook to find the "__has__" member, but the JavaAdapter here 
> does not override __call__, and then the lookup will fail.
>
> Rather than going through the trouble of implementing the __call__ 
> hook just for this purpose, I move the __has__ function up, and made 
> __get__ call __has__ directly instead. Now it won't trigger the 
> __call__ hook for the lookup, the things will work fine again.
>
> Line 45-48:
>
> Removed trailing whitespace.
>
> Line 84-85:
>
> This code used to work in Rhino, but apparently Nashorn doesn't 
> automatically convert a JavaScript Array into a Java array when doing 
> JS-to-Java interop, so this conversion has to be done manually.
>
> Line 93-104:
>
> I'm a little confused in this part. Nashorn exposes a "print" function 
> that's defined in jdk/nashorn/api/scripting/resources/engine.js, and 
> that it doesn't expose a corresponding "println" function. I'm not 
> sure if this code really worked when using Rhino...anyway, the 
> "println" function is missing, so "writeln = println" doesn't do 
> anything useful. I'm adding a shim here just in case either of "print" 
> or "println" functions are missing. This fixes an error when calling 
> "println" in line 87 of this patch.
>
> Line 113:
>
> This is the same change as purposed by Yunda back in April. [1] A 
> missing fix from the NPG changes.
>
> Line 121-122, 129-130:
>
> When specifying a method overload in Nashorn, the argument syntax it 
> takes for inner classes uses "." as the separator between the 
> enclosing class name and the inner class name, instead of "$" as in 
> the "binary name".
>
> Line 138-144, 152-160:
>
> Nashorn has stricter default behaviors than Rhino when overriding Java 
> methods. Where as Rhino defaults to a "do nothing" implementation, 
> Nashorn defaults to throwing UnsupportedOperationException for 
> unimplemented methods.
>
> Line 168-172:
>
> Before the fix, this code only replace the first occurrence of the 
> specified special characters, which happens to be not enough for newer 
> HotSpot types in C++ templates.
>
> That's all for this patch.
>
> BTW, there is another patches pending review, too: JDK-8011979 [2]
>
> Thanks,
> Kris
>
> [1]: 
> http://mail.openjdk.java.net/pipermail/serviceability-dev/2013-April/009043.html
> [2]: 
> http://mail.openjdk.java.net/pipermail/serviceability-dev/2013-April/009149.html



More information about the serviceability-dev mailing list