JDK-8137333 : Boundless soft caching of property map histories causes high memory pressure
  • Type: Bug
  • Component: core-libs
  • Sub-Component: jdk.nashorn
  • Affected Version: 8u40,9
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • OS: os_x
  • CPU: x86
  • Submitted: 2015-09-25
  • Updated: 2016-01-14
  • Resolved: 2015-09-30
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 b85Fixed
Related Reports
Duplicate :  
JDK-8141210 - Very slow loading of JavaScript file with recent JDK
Relates :  
JDK-8043605 - Enable history for empty property maps
Description
FULL PRODUCT VERSION :


A DESCRIPTION OF THE PROBLEM :
We use Nashorn to run the zxcvb javascript, which is a password strength checker.
We do that to perform the same serverside validation as in the webpage.
There is a sample repository at
https://github.com/bripkens/nashorn-pw-check-example
which contains a unit test https://github.com/bripkens/nashorn-pw-check-example/blob/master/src/test/java/de/codecentric/StrengthCheckerTest.java 
This unit test takes about 7 seconds to complete when run with a java 8 u 20.

Running it with 8u40 or 9ea spins forever in internal Nashorn code.

While it is possible tht this behaviour is triggered by the zxcvb library, as a user, I consider it to be a regression when it worked in u20 and no longer works in u40.

REGRESSION.  Last worked in version 8u40

ADDITIONAL REGRESSION INFORMATION: 
worked in u20, does NOT work in u40 (the select box does not allow this combination)

STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
clone the repo. run maven install with the desired jvms, or run mvn eclipse:eclipse and try to run from eclipse with the given jdks.

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
unit tests pass.
ACTUAL -
spins forever

REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
https://github.com/bripkens/nashorn-pw-check-example/
---------- END SOURCE ----------
Comments
No issues with the nightly where the fix is integrated. As far as this issue has high impact we may take it in PSU15_04.
05-10-2015
Critical Request Template - Justification : Serious performance regression compared to 8u31; exists since 8u40. It renders Nashorn unable to run the popular JS-based password strength checker library named "zxcvbn.js" developed by Dropbox. - Risk Analysis : No risk. Very small change. The fix introduces changing of caching for JavaScript derived hidden classes from soft-referenced caching to hybrid caching that starts out as soft and changes to weak after a threshold. Before only soft references were used, and as the VM tended to retain them it was possible to bring it into a state where it was spending inordinate amount of times in the GC. - Webrev : http://mail.openjdk.java.net/pipermail/nashorn-dev/2015-September/005341.html - Testing (done/to-be-done) : The test and test262 test suites have been run and indicated 0 failures. Running the https://github.com/bripkens/nashorn-pw-check-example/blob/master/src/test/java/de/codecentric/StrengthCheckerTest.java 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
30-09-2015
So the difference is that we're starting histories from empty property map since JDK-8043605, and zxcvbn.js has a "q" function that dynamically populates an object with +10k properties starting from an empty object {}. Switching to weak references from soft references solves the problem, but it would have a small performance penalty if small objects are dynamically populated repeatedly with the same properties. So we adopted an approach where the PropertyMap carries a "derivation count" counting how many times it was derived from another property map, and when it reaches a threshold (by default 32, but we made it configurable with the system property named "nashorn.propertyMap.softReferenceDerivationLimit") it and all maps derived from it start using weak references for their history. Therefore, we softly cache up to 32 property adds/deletes originating from a root property map and afterwards we switch to weakly referencing them; they'll still remain alive if they are bound by named setters, otherwise they'll get cleaned up quickly by the GC.
30-09-2015
Investigation so far shows that we'll retain a long chain of PropertyMap history as the "q" function creates objects populated with >10k entries, added one by one. This will take up around 1.3GB (!) of memory. What's weird is that memory analysis shows that these are rooted in compiled script's constants[] array, in its 0th entry, which is indeed the map for the "d = {}" initial object. However, the same root PropertyMap is added to the code even if I roll back the code to 8u20 state. For some reason though, the maps end up being cleared much more eagerly in 8u20. It can't be that the properties became unreachable earlier in 8u20 as they're all, in the end, bound into the latest map of the object being created. Changing from using SoftReference to WeakReference to hold on to the history elements fixes the problem, although I still don't understand why (or rather, why is this not a problem in 8u20). It does actually seem that using WeakReference should be sufficient to deduplicate maps that are otherwise still in use. (In general case they'll also be strongly-referenced as bound parameters of named property setter call sites.) In this case though, an indexed setter is being used, so the interim maps are indeed not reachable strongly. I'm still at loss as to what is the difference between 8u20 and later versions that makes the difference in behaviour.
29-09-2015
Bisected the jdk9 repo and it identified JDK-8043605 "Enable history for empty property maps" as the cause for this. Sounds very plausible.
29-09-2015