RFR: 8337505: Footprint and startup regressions up to 20% in GUI apps

[Aleksey Shipilev]

On Wed, 4 Dec 2024 18:47:52 GMT, Phil Race <prr at openjdk.org> wrote:

> Thanks for the suggestion, but it isn't a path I want to go. The patch is still fine as it is.

Well, the point of the review process is to get at least some reviewers to agree. I do not see anyone agreeing so far.

My problem with current version is that exposing non-constant-foldable `Method/VarHandle`-s is a performance anti-pattern. I poked around the font code, and I have not been able to convince myself these accesses are never on the hot-path. I see this is also a _cache_, so I would think there is an expectation that most, if not all things in it work as fast as reasonably possible. Therefore, I cannot put my approval on current version, sorry.

I do not quite understand what is the problem with stamping `@Stable` over the now-non-`final` fields in your current PR, like below? Is this only about the conceptual uncleanliness of exposing `@Stable` to `java.desktop`, or is it about something else?


diff --git a/src/java.base/share/classes/module-info.java b/src/java.base/share/classes/module-info.java
index c3bcbed4e3b..66ffff5723a 100644
--- a/src/java.base/share/classes/module-info.java
+++ b/src/java.base/share/classes/module-info.java
@@ -261,6 +261,7 @@
         jdk.internal.vm.ci,
         jdk.jfr;
     exports jdk.internal.vm.annotation to
+        java.desktop,
         java.instrument,
         jdk.internal.vm.ci,
         jdk.incubator.vector,
diff --git a/src/java.desktop/share/classes/sun/font/StrikeCache.java b/src/java.desktop/share/classes/sun/font/StrikeCache.java
index e71235d54c0..5271afbe611 100644
--- a/src/java.desktop/share/classes/sun/font/StrikeCache.java
+++ b/src/java.desktop/share/classes/sun/font/StrikeCache.java
@@ -43,6 +43,8 @@
 import java.lang.ref.WeakReference;
 import java.util.*;
 
+import jdk.internal.vm.annotation.Stable;
+
 import sun.java2d.Disposer;
 import sun.java2d.pipe.BufferedContext;
 import sun.java2d.pipe.RenderQueue;
@@ -136,16 +138,16 @@ private static VarHandle getVarHandle(StructLayout struct, String name) {
         return MethodHandles.insertCoordinates(h, 1, 0L).withInvokeExactBehavior();
     }
 
-    private static VarHandle xAdvanceHandle;
-    private static VarHandle yAdvanceHandle;
-    private static VarHandle widthHandle;
-    private static VarHandle heightHandle;
-    private static VarHandle rowBytesHandle;
-    private static VarHandle managedHandle;
-    private static VarHandle topLeftXHandle;
-    private static VarHandle topLeftYHandle;
-    private static VarHandle cellInfoHandle;
-    private static VarHandle imageHandle;
+    @Stable private static VarHandle xAdvanceHandle;
+    @Stable private static VarHandle yAdvanceHandle;
+    @Stable private static VarHandle widthHandle;
+    @Stable private static VarHandle heightHandle;
+    @Stable private static VarHandle rowBytesHandle;
+    @Stable private static VarHandle managedHandle;
+    @Stable private static VarHandle topLeftXHandle;
+    @Stable private static VarHandle topLeftYHandle;
+    @Stable private static VarHandle cellInfoHandle;
+    @Stable private static VarHandle imageHandle;
 
     private static VarHandle getXAdvanceHandle() {
         if (xAdvanceHandle == null) {

-------------

PR Comment: https://git.openjdk.org/jdk/pull/21748#issuecomment-2519724561