JDK-8153151 : Call JavaScriptCore GC whenever JVM GC happens
  • Type: Bug
  • Component: javafx
  • Sub-Component: web
  • Affected Version: 9
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2016-03-31
  • Updated: 2016-06-23
  • Resolved: 2016-04-07
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
8u102Fixed 9Fixed
Description
Few JVM heap objects(ImageDecoder, Font,..) are held by JavaScript garbages. It is better to call speculatively JSC's GC whenever JVM GC happens.
Comments
Changeset: 84e09aadadf1 Author: arajkumar Date: 2016-04-07 13:41 -0700 URL: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/84e09aadadf1
07-04-2016
Looks good to me.
07-04-2016
Incorporated review comments: http://cr.openjdk.java.net/~arajkumar/8153151/webrev.01 Other than this, I took a time to GC in maps.google.com: No of samples: 62 min time to complete GC: 2.377 ms max time to complete GC: 72.814 ms Average time to GC : 27.36609677 ms
07-04-2016
>> 1. You need to add the new native JNI method ito the map files as indicated above. Thanks. I will add the newly exported symbol into mapfiles. >>2. I note that you are relying on garbage-collecting a small, short-lived object that you create without holding a reference to it. This could lead to the collectJSCGarbages method being called either more or less frequently than you might expect (or want). Have you considered the possibility of calling the JS GC when a WebPage is destroyed? Or are you worried that is not often enough in a situation where there is limited memory? I am OK with the current approach, though, as long as you have considered alternatives. Calling from WebPage.dispose() is not frequent enough to clear the garbage. Most of the garbage are generated during page interaction, that is the reason for using Disposer based hack. >>3. Please add a blank line between the native variable and the method: >>+ private static boolean firstWebPageCreated = false; >>+ protected static void collectJSCGarbages() { >>Also, there is no reason for the static method to be protected. The default (package scope) can/should be used. Will change according style guideline. Thank you @Kevin.
07-04-2016
Looks good to me, too. I will do one last sanity check build and then push it.
07-04-2016
Following up on point #3 above, I think the collectJSCGarbages method can actually be private.
06-04-2016
Overall, the fix looks fine, and I tested that there is only ever one queued up Disposer request at a time. I do have one question and a couple comments. 1. You need to add the new native JNI method ito the map files as indicated above. 2. I note that you are relying on garbage-collecting a small, short-lived object that you create without holding a reference to it. This could lead to the collectJSCGarbages method being called either more or less frequently than you might expect (or want). Have you considered the possibility of calling the JS GC when a WebPage is destroyed? Or are you worried that is not often enough in a situation where there is limited memory? I am OK with the current approach, though, as long as you have considered alternatives. 3. Please add a blank line between the native variable and the method: + private static boolean firstWebPageCreated = false; + protected static void collectJSCGarbages() { Also, there is no reason for the static method to be protected. The default (package scope) can/should be used.
06-04-2016
I get the following exception when running WebView on Mac with this fix: Exception in thread "JavaFX Application Thread" java.lang.UnsatisfiedLinkError: com.sun.webkit.WebPage.twkDoJSCGarbageCollection()V at com.sun.webkit.WebPage.twkDoJSCGarbageCollection(Native Method) at com.sun.webkit.WebPage.collectJSCGarbages(WebPage.java:152) at com.sun.webkit.WebPage$$Lambda$73/1429617200.dispose(Unknown Source) at com.sun.webkit.Disposer$WeakDisposerRecord.dispose(Disposer.java:163) at com.sun.webkit.Disposer$DisposerRunnable.run(Disposer.java:139) This is because the added JNI symbols are not reflected in the --- modules/web/src/main/native/Source/WebCore/mapfile-macosx and modules/web/src/main/native/Source/WebCore/mapfile-vers files.
06-04-2016
[~arajkumar] Do you want to add log just before calling twkDoJSCGarbageCollection ? (not mandatory) Rest of the changes are fine.. +1
01-04-2016
http://cr.openjdk.java.net/~arajkumar/8153151/webrev.00/
01-04-2016