JDK-8133785 : SharedScopeCall should be enabled for non-optimistic call sites even with optimistic compilation
  • Type: Enhancement
  • Component: core-libs
  • Sub-Component: jdk.nashorn
  • Affected Version: 9
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2015-08-18
  • Updated: 2016-01-14
  • Resolved: 2015-08-24
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 8 JDK 9
8u72Fixed 9 b80Fixed
Related Reports
Relates :  
Description
Currently, SharedScopeCall is disabled when doing optimistic compilation. We plan to rectify this (see JDK-8069338), but it is non-trivial. In the meantime, we can partially implement it and go long ways towards reclaiming its performance benefits: enable it during optimistic compilation for those call sites that end up being non-optimistic (that is, their statically proven return type is Object)
Comments
IMPLEMENTATION NOTES: The essence of the change is contained in CodeGenerator.java. Even there I tidied up three things in addition to the core change (but related to it): I eliminated the fnIdToContinuationInfo map, simplified loadSharedScopeVar to assert isFastScope (it is always invoked with isFastScope already tested) and removed isOptimisticOrRestOf method, as it is unused. All the rest of the change does with disabling the combination of eager compilation and optimistic types. This combination was never useful, but now with optimistic typing allowing scope calls, it became actively hurtful. The change ended being somewhat nontrivial though, as per below: - ScriptEnvironment now throws an IllegalStateException when --lazy-compilation=false and --optimistic-types=true - In order to report a meaningful error message without hard coding command line options in it, I added Options.getOptionTemplateByKey method. I also clarified the names of Options.getOptionTemplateByName and OptionTempplate.nameMatches as the way they were coded was confusing (e.g. if they're operating with key or name). - I added a system property nashorn.options.allowEagerCompilationSilentOverride that instead of throwing an exception in the case above silently switches back to --lazy-compilation=true. This is subsequently used in tests that explicitly turn lazy compilation off, so that they still work in the optimistic-types part of the test matrix.
20-08-2015

From the "subtle dependencies" department: turns out that if we start implementing shared scope calls for optimistic compilation, then it'll be hard to make --optimistic-types=true work with --lazy-compilation=false. Currently the test/script/basic/JDK-8053905.js fails for the subtle reason that with eager compilation we have globally correct Symbol.useCount as the script is compiled at once, while recompilations (which are by definition lazy, or rather just-in-time) will only count symbol use local to that functions. Therefore, we can have a discrepancy in whether a Symbol hits the threshold for shared scope calls in eager compilation (hits) and in later recompilations (does not hit), resulting in different stack shapes, causing a stack shape mismatch for a rest-of method��� The point where JDK-8053905.js fails is earley-boyer's sc_member function which invokes sc_isEqual in line 1000. In eager compilation, sc_isEqual qualifies for a shared scope call, since it is used 10 times globally in the script (threshold is 4). In a subsequent deoptimizing recompilation of sc_member it is only used once, so it will not be invoked through a shared scope call. We could fix this using several strategies: - ignoring --lazy-compilation=false when --optimistic-compilation=true. This is least effort and would fix the issue��� We should allow it but log a warning, though. (We could also throw an error for incompatible flags, but then we'd have more trouble configuring tests that explicitly use --lazy-compilation=false for testing with optimistic and pessimistic) - throwing away eager compilation altogether. Maybe too drastic at this point, but could be an option going forward. - making sure that recompilations also see the global symbol count. For this, we would have to store symbol counts in RecompilableScriptFunctionData, gathered at initial compilation, similar to how today we do for scope depth, but this would be quite hard to get correctly (symbol shadowing across scopes, symbol local to recompiled function but also used from a nested function; symbol external to recompiled function; symbol external to recompiled function but also used from a nested function���)
18-08-2015

After implementing this strategy, the number of call sites in mandreel.js optimistic execution goes down from 344395 to 36545 (yes, about 90% reduction!). For comparison, the number of call sites in non-optimistic execution is 19563. Optimistic adds another ~17k call sites, mostly because it recompiles. A run with 2 iterations (warmup + 2 iterations) also goes down from 1m38s to 1m20-1m24s (with some variability) on my machine, and warms up much faster: before the change the warmup round was 1 op/minute, now it's 3 op/minute.
18-08-2015