JDK-8134403 : Nashorn react.js benchmark performance regression
  • Type: Bug
  • Component: core-libs
  • Sub-Component: jdk.nashorn
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2015-08-25
  • Updated: 2016-01-14
  • Resolved: 2015-08-26
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
8u66Fixed 9 b80Fixed
Related Reports
Relates :  
JDK-8062401 - User accessors require boxing and do not support optimistic types
Description
https://github.com/maximenajim/java-vs-node-react-rendering-microbenchmark

This benchmark is reported to exhibit a significant performance regression that was introduced between 8u31 and 8u40.

Running the benchmark with -Xprof indicates a ca. 88 % profile for java.lang.invoke.MethodHandleNatives.setCallSiteTargetNormal; a JFR run (-XX:+UnlockCommercialFeatures -XX:+FlightRecorder -XX:FlightRecorderOptions=defaultrecording=true,disk=true,dumponexit=true,dumponexitpath=react.jfr,stackdepth=1024) indicates megamorphism around calls to "construct" functions from "instantiateReactComponent" functions.

In principle, this can be due to either Nashorn or java.lang.invoke changes in between 8u31 and 8u40.

JFR log is attached.
Comments
SQE OK to take the fix to PSU15_04
31-08-2015
Does it make sense to ask perf team to add mentioned benchmark to list of suites that can be run against Nashorn via regular infrastructure? If yes let's file an enhancement to track it.
27-08-2015
Critical Request Template - Justification : Serious performance regression compared to 8u31; exists since 8u40. - Risk Analysis : No risk. Very small change. The fix introduces stable caching for a method handle that was previously recreated repeatedly. It basically restores the situation to what was in place in 8u31. Most importantly, the change also preserves the essential improvement of JDK-8062401 which caused the regression. - Webrev : http://mail.openjdk.java.net/pipermail/nashorn-dev/2015-August/005082.html - Testing (done/to-be-done) : The test and test262 test suites have been run and indicated 0 failures. Running the https://github.com/maximenajim/java-vs-node-react-rendering-microbenchmark benchmark that uncovered the regression confirms that performance was regained. - Back ports (done/to-be-done) : Backported to 8u-dev; 8u-dev backport needs to be pushed into 8u66 if approved here. No further backports necessary. - FX Impact : N/A - Fix For Release : 8u66
27-08-2015
"element.props" getter at react.js:4971 becomes megamorphic, at which point we start invoking it through ScriptObject.megamorphicGet(). At least one of "element.props" evaluates to a getter, namely the one defined in react.js:9327 ("defineWarningProperty"). Unfortunately, since JDK-8062401 we don't cache dynamic invokers for getters in UserAccessorProperty, so every get will cause re-bootstrapping of a new call site. The solution for this would be to recognize two usages of getter invokers: one when linked into a getter (UserAccessorProperty.getGetter() - we could probably keep creating a new one for each invocation) and one cached instance when used for direct invocation from UserAccessorProperty.getObjectValue().
26-08-2015
The execution hotspot is UserAccessorProperty.getINVOKE_UA_GETTER()
26-08-2015