8u20 with multi-threaded class cache

[Chris Pettitt]

Hi Sundar,

Thanks again for all of the help. The code looks good when I pulled from jdk8u-dev.

Thanks,
Chris

________________________________________
From: A. Sundararajan [sundararajan.athijegannathan at oracle.com]
Sent: Thursday, March 19, 2015 11:10
To: Chris Pettitt; nashorn-dev at openjdk.java.net
Subject: Re: 8u20 with multi-threaded class cache

Hi,

Perhaps repo not sync'ed?

http://hg.openjdk.java.net/jdk8u/jdk8u/nashorn/file/da9741520576/src/jdk/nashorn/api/scripting/NashornScriptEngine.java

You can see Source.sourceFor static helper is called to create Source
object. And that one has cache logic to choose existing Source - if
available. And Context.findCachedClass(Source) can check Source->Class
cache.

BTW, nashorn development checks into this repo for 8 update fixes:

http://hg.openjdk.java.net/jdk8u/jdk8u-dev/

You can check here too:

http://hg.openjdk.java.net/jdk8u/jdk8u-dev/nashorn/file/e024db176497/src/jdk/nashorn/api/scripting/NashornScriptEngine.java

Hope this helps,
-Sundar


On Thursday 19 March 2015 11:05 PM, Chris Pettitt wrote:
> Hi Sundar,
>
> Thanks for the follow up. I'm hoping for something along the lines of what you
> described. The code I see seems to suggest different behavior (described
> below), but it could be that I'm looking at older code (I'm using [1]) or that
> I'm just looking at the wrong code.
>
> Let's assume we'll use the same URLReader for each call to engine.eval. The
> call to engine.eval will then call makeSource:
>
>      @Override
>      public Object eval(final Reader reader, final ScriptContext ctxt) throws ScriptException {
>          return evalImpl(makeSource(reader, ctxt), ctxt);
>      }
>
> This in turn creates a new Source - at least in the version I'm looking at:
>
>      private static Source makeSource(final Reader reader, final ScriptContext ctxt) throws ScriptException {
>          try {
>              if (reader instanceof URLReader) {
>                  final URL url = ((URLReader)reader).getURL();
>                  final Charset cs = ((URLReader)reader).getCharset();
>                  return new Source(url.toString(), url, cs);
>              }
>              return new Source(getScriptName(ctxt), Source.readFully(reader));
>          } catch (final IOException e) {
>              throw new ScriptException(e);
>          }
>      }
>
> Since we created a new object for the Source, the reference check in
> Source.equals will return false, and we'll fall back to doing a full array
> comparison:
>
>      @Override
>      public boolean equals(final Object obj) {
>          if (this == obj) {
>              return true;
>          }
>
>          if (!(obj instanceof Source)) {
>              return false;
>          }
>
>          final Source src = (Source)obj;
>          // Only compare content as a last resort measure
>          return length == src.length && Objects.equals(url, src.url) && Objects.equals(name, src.name) && Arrays.equals(content, src.content);
>      }
>
> Also, by virtue of calling Source(url.toString(), url, cs) we'll end up
> re-reading the entire source each time we call eval:
>
>      public Source(final String name, final URL url, final Charset cs) throws IOException {
>          this(name, baseURL(url, null), readFully(url, cs), url);
>      }
>
> What do you think? Am I looking in the wrong place, out-of-date
> code, or is my understanding flawed?
>
> Thanks again to both you and Hannes for your patience and help.
>
> [1]: http://hg.openjdk.java.net/jdk8u/jdk8u
>
> Best,
> Chris
>
> ________________________________________
> From: nashorn-dev [nashorn-dev-bounces at openjdk.java.net] on behalf of A. Sundararajan [sundararajan.athijegannathan at oracle.com]
> Sent: Thursday, March 19, 2015 06:32
> To: nashorn-dev at openjdk.java.net
> Subject: Re: 8u20 with multi-threaded class cache
>
> Hi Chris,
>
> If you use Nashorn's URLReader (
> http://docs.oracle.com/javase/8/docs/jdk/api/nashorn/jdk/nashorn/api/scripting/URLReader.html
> - a Reader wrapping a script URL object), then nashorn's Source would be
> cached against that URL could be re-used (and the compiled Class object
> as well via findCachedClass).
>
>       engine.eval(new URLReader(myScriptURL));
>
> Also, any script loaded via "load(url)" built-in could also be
> compiled/cached and re-used (against that URL + last modified since of
> the same).
>
> Hope this helps,
> -Sundar
>
> On Thursday 19 March 2015 04:00 AM, Chris Pettitt wrote:
>> Hannes,
>>
>> Thanks for the detailed reply - this is very helpful. Your answers clear up my questions for #1 and #2.
>>
>> For #3, I was hoping we could use some key to retrieve the cached classes that did not involve reloading / re-reading the source scripts on each lookup. jdk.nashorn.internal.runtime.Source is used as the key in jdk.nashorn.internal.objects.Global.findCachedClass. Source tries to avoid the cost of walking the source contents twice, once for hashing (by caching the hash) and once for the equality check by checking the reference. Unfortunately, it *appears* that we're always locked out of this optimization because there is no public way to feed a Source into the NashornScriptEngine.
>>
>> Thanks,
>> Chris
>>
>> ________________________________________
>> From: Hannes Wallnoefer [hannes.wallnoefer at oracle.com]
>> Sent: Tuesday, March 17, 2015 10:38
>> To: Chris Pettitt; nashorn-dev at openjdk.java.net; Kunal Cholera
>> Subject: Re: 8u20 with multi-threaded class cache
>>
>> Hi Chris, Kunal,
>>
>> Answers are inlined below.
>>
>> Am 2015-03-16 um 23:03 schrieb Chris Pettitt:
>>> Hi folks,
>>>
>>> We're looking into the possibility of using the class cache in multi-threaded code, as introduced in [1]. We have a few questions related to this feature:
>>>
>>> 1. The article implies that ScriptEngine can be treated as thread-safe - provided we're not using the default context - though the code doesn't state this explicitly. Is this a safe assumption? Are there any other caveats?
>> This is correct as far as Nashorn is concerned. In the original JDK8
>> release, using a ScriptEngine with multiple bindings/globals will
>> compile each script from scratch. We introduced the class caching in
>> 8u20 as our best effort to fit code reuse on the existing ScriptEngine
>> API. The only caveat I can think of is that code may run slower with
>> multiple bindings because of callsite polymorphism. Also bear in mind
>> that this is a relatively new feature, but there seemed to be no problem
>> using it with your dust scripts (see example below).
>>
>>> 2. As we need to set the Context for each eval, does this lock us out of using Invocable?
>> You can actually use this with Invocable.invokeMethod(), passing the
>> binding as first argument ("thiz" parameter). I've rewritten the
>> threaded class cache example from the blog post to do this with your
>> dust benchmark and it seems to work fine:
>>
>> https://gist.github.com/hns/8f52a620ce36daa3d0ca
>>
>> I just edited the bench.js file to remove the benchmark loop at the
>> bottom, otherwise this will run with the script files you sent me. Feel
>> free to use this as a starting point for your own testing.
>>
>> It could also work with the other Invocable method taking a "thiz"
>> parameter, getInterface(Object, Class), but I haven't tested this.
>>
>>> 3. The code for determining if two Sources are the same ultimately falls back to a comparison of the url / name / content of the scripts. Is there a way to eval with a Source to avoid this fallback? It looks like it is not exposed in a public way.
>> I'm not sure I understand this question. What I use in my example is
>> jdk.nashorn.api.scripting.URLReader, which is handy to preserve the
>> source URL in error messages.  I believe Nashorn will still load the
>> content of the URL to make sure scripts are actually identical.
>>
>> I hope this helps. Let me know if you have further questions.
>>
>> Regards,
>> Hannes
>>
>>> [1]: https://blogs.oracle.com/nashorn/entry/improving_nashorn_startup_time_using
>>>
>>> Thanks,
>>> Chris