Merge pull request scala#9571 from dwijnand/rework-AlmostFinalValue

Rework AlmostFinalValue, to make it as fast as possible

Author: retronym

build.sbt

@@ -664,12 +664,13 @@ lazy val bench = project.in(file("test") / "benchmarks")
     name := "test-benchmarks",
     autoScalaLibrary := false,
     crossPaths := true, // needed to enable per-scala-version source directories (https://github.com/sbt/sbt/pull/1799)
+    compileOrder := CompileOrder.JavaThenScala, // to allow inlining from Java ("... is defined in a Java source (mixed compilation), no bytecode is available")
     libraryDependencies += "org.openjdk.jol" % "jol-core" % "0.10",
     libraryDependencies ++= {
       if (benchmarkScalaVersion == "") Nil
       else "org.scala-lang" % "scala-compiler" % benchmarkScalaVersion :: Nil
     },
-    scalacOptions ++= Seq("-feature", "-opt:l:inline", "-opt-inline-from:scala.**")
+    scalacOptions ++= Seq("-feature", "-opt:l:inline", "-opt-inline-from:scala/**", "-opt-warnings"),
   ).settings(inConfig(JmhPlugin.JmhKeys.Jmh)(scalabuild.JitWatchFilePlugin.jitwatchSettings))
 
 

src/reflect/scala/reflect/internal/util/AlmostFinalValue.java

@@ -14,93 +14,35 @@
 
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
-import java.lang.invoke.MethodType;
 import java.lang.invoke.MutableCallSite;
-import java.lang.invoke.SwitchPoint;
 
 /**
  * Represents a value that is wrapped with JVM machinery to allow the JVM
- * to speculate on its content and effectively optimize it as if it was final.
- * 
- * This file has been drawn from JSR292 cookbook created by Rémi Forax.
- * https://code.google.com/archive/p/jsr292-cookbook/. The explanation of the strategy
- * can be found in https://community.oracle.com/blogs/forax/2011/12/17/jsr-292-goodness-almost-static-final-field.
- * 
- * Before copying this file to the repository, I tried to adapt the most important
- * parts of this implementation and special case it for `Statistics`, but that
- * caused an important performance penalty (~10%). This performance penalty is
- * due to the fact that using `static`s for the method handles and all the other
+ * to speculate on its content and effectively optimize it as if it was a constant.
+ *
+ * Originally from the JSR-292 cookbook created by Rémi Forax:
+ * https://code.google.com/archive/p/jsr292-cookbook/.
+ *
+ * Implemented in Java because using `static`s for the method handles and all the other
  * fields is extremely important for the JVM to correctly optimize the code, and
  * we cannot do that if we make `Statistics` an object extending `MutableCallSite`
- * in Scala. We instead rely on the Java implementation that uses a boxed representation.
+ * in Scala.
+ *
+ * Subsequently specialised for booleans, to avoid needless Boolean boxing.
+ *
+ * Finally reworked to default to false and only allow for the value to be toggled on,
+ * using Rémi Forax's newer "MostlyConstant" as inspiration, in https://github.com/forax/exotic.
  */
-public class AlmostFinalValue {
-  private final AlmostFinalCallSite callsite =
-      new AlmostFinalCallSite(this);
-  
-  protected boolean initialValue() {
-    return false;
-  }
-  
-  public MethodHandle createGetter() {
-    return callsite.dynamicInvoker();
-  }
-  
-  public void setValue(boolean value) {
-    callsite.setValue(value);
-  }
-  
-  private static class AlmostFinalCallSite extends MutableCallSite {
-    private Boolean value;
-    private SwitchPoint switchPoint;
-    private final AlmostFinalValue volatileFinalValue;
-    private final MethodHandle fallback;
-    private final Object lock;
-    
-    private static final Boolean NONE = null;
-    private static final MethodHandle FALLBACK;
-    static {
-      try {
-        FALLBACK = MethodHandles.lookup().findVirtual(AlmostFinalCallSite.class, "fallback",
-            MethodType.methodType(Boolean.TYPE));
-      } catch (NoSuchMethodException|IllegalAccessException e) {
-        throw new AssertionError(e.getMessage(), e);
-      }
-    }
-    
-    AlmostFinalCallSite(AlmostFinalValue volatileFinalValue) {
-      super(MethodType.methodType(Boolean.TYPE));
-      Object lock = new Object();
-      MethodHandle fallback = FALLBACK.bindTo(this);
-      synchronized(lock) {
-        value = null;
-        switchPoint = new SwitchPoint();
-        setTarget(fallback);
-      }
-      this.volatileFinalValue = volatileFinalValue;
-      this.lock = lock;
-      this.fallback = fallback;
-    }
+final class AlmostFinalValue {
+  private static final MethodHandle K_FALSE = MethodHandles.constant(boolean.class, false);
+  private static final MethodHandle K_TRUE  = MethodHandles.constant(boolean.class, true);
+
+  private final MutableCallSite callsite = new MutableCallSite(K_FALSE);
+          final MethodHandle    invoker  = callsite.dynamicInvoker();
 
-    boolean fallback() {
-      synchronized(lock) {
-        Boolean value = this.value;
-        if (value == NONE) {
-          value = volatileFinalValue.initialValue();
-        }
-        MethodHandle target = switchPoint.guardWithTest(MethodHandles.constant(Boolean.TYPE, value), fallback);
-        setTarget(target);
-        return value;
-      }
-    }
-    
-    void setValue(boolean value) {
-      synchronized(lock) {
-        SwitchPoint switchPoint = this.switchPoint;
-        this.value = value;
-        this.switchPoint = new SwitchPoint();
-        SwitchPoint.invalidateAll(new SwitchPoint[] {switchPoint});
-      }
-    }
+  void toggleOnAndDeoptimize() {
+    if (callsite.getTarget() == K_TRUE) return;
+    callsite.setTarget(K_TRUE);
+    MutableCallSite.syncAll(new MutableCallSite[] { callsite });
   }
-}
+}

src/reflect/scala/reflect/internal/util/Statistics.scala

@@ -301,7 +301,7 @@ quant)
   @inline final def enabled: Boolean = areColdStatsLocallyEnabled
   def enabled_=(cond: Boolean) = {
     if (cond && !enabled) {
-      StatisticsStatics.enableColdStats()
+      StatisticsStatics.enableColdStatsAndDeoptimize()
       areColdStatsLocallyEnabled = true
     }
   }
@@ -310,7 +310,7 @@ quant)
   @inline final def hotEnabled: Boolean = enabled && areHotStatsLocallyEnabled
   def hotEnabled_=(cond: Boolean) = {
     if (cond && enabled && !areHotStatsLocallyEnabled) {
-      StatisticsStatics.enableHotStats()
+      StatisticsStatics.enableHotStatsAndDeoptimize()
       areHotStatsLocallyEnabled = true
     }
   }

src/reflect/scala/reflect/internal/util/StatisticsStatics.java

@@ -12,7 +12,6 @@
 
 package scala.reflect.internal.util;
 
-import scala.reflect.internal.util.AlmostFinalValue;
 import java.lang.invoke.MethodHandle;
 
 /**
@@ -22,46 +21,15 @@
  * which helps performance (see docs to find out why).
  */
 public final class StatisticsStatics {
-  private static final AlmostFinalValue COLD_STATS = new AlmostFinalValue() {
-    @Override
-    protected boolean initialValue() {
-        return false;
-    }
-  };
+  private static final AlmostFinalValue COLD_STATS = new AlmostFinalValue();
+  private static final AlmostFinalValue  HOT_STATS = new AlmostFinalValue();
 
-  private static final AlmostFinalValue HOT_STATS = new AlmostFinalValue() {
-    @Override
-    protected boolean initialValue() {
-        return false;
-    }
-  };
+  private static final MethodHandle COLD_STATS_GETTER = COLD_STATS.invoker;
+  private static final MethodHandle  HOT_STATS_GETTER =  HOT_STATS.invoker;
 
-  private static final MethodHandle COLD_STATS_GETTER = COLD_STATS.createGetter();
-  private static final MethodHandle HOT_STATS_GETTER = HOT_STATS.createGetter();
-  
-  public static boolean areSomeColdStatsEnabled() throws Throwable {
-    return (boolean) COLD_STATS_GETTER.invokeExact();
-  }
+  public static boolean areSomeColdStatsEnabled() throws Throwable { return (boolean) COLD_STATS_GETTER.invokeExact(); }
+  public static boolean areSomeHotStatsEnabled()  throws Throwable { return (boolean)  HOT_STATS_GETTER.invokeExact(); }
 
-  public static boolean areSomeHotStatsEnabled() throws Throwable {
-    return (boolean) HOT_STATS_GETTER.invokeExact();
-  }
-
-  public static void enableColdStats() throws Throwable {
-    if (!areSomeColdStatsEnabled())
-      COLD_STATS.setValue(true);
-  }
-
-  public static void disableColdStats() {
-    COLD_STATS.setValue(false);
-  }
-
-  public static void enableHotStats() throws Throwable {
-    if (!areSomeHotStatsEnabled())
-      HOT_STATS.setValue(true);
-  }
-
-  public static void disableHotStats() {
-    HOT_STATS.setValue(false);
-  }
+  public static void enableColdStatsAndDeoptimize() { COLD_STATS.toggleOnAndDeoptimize(); }
+  public static void  enableHotStatsAndDeoptimize() {  HOT_STATS.toggleOnAndDeoptimize(); }
 }

test/benchmarks/src/main/java/scala/reflect/internal/util/AlmostFinalValueBenchmarkStatics.java

@@ -0,0 +1,12 @@
+package scala.reflect.internal.util;
+
+import java.lang.invoke.MethodHandle;
+
+final class AlmostFinalValueBenchmarkStatics {
+  static final boolean STATIC_FINAL_FALSE = false;
+
+  private static final AlmostFinalValue ALMOST_FINAL_FALSE    = new AlmostFinalValue();
+  private static final MethodHandle ALMOST_FINAL_FALSE_GETTER = ALMOST_FINAL_FALSE.invoker;
+
+  static boolean isTrue() throws Throwable { return (boolean) ALMOST_FINAL_FALSE_GETTER.invokeExact(); }
+}

test/benchmarks/src/main/scala/scala/reflect/internal/util/AlmostFinalValueBenchmark.scala

@@ -0,0 +1,56 @@
+package scala.reflect.internal.util
+
+import java.util.concurrent.TimeUnit
+
+import org.openjdk.jmh.annotations._
+import org.openjdk.jmh.infra.Blackhole
+
+class AlmostFinalValueBenchSettings extends scala.reflect.runtime.Settings {
+  val flag = new BooleanSetting(false)
+
+  @inline final def isTrue2: Boolean = AlmostFinalValueBenchmarkStatics.isTrue && flag
+}
+
+object AlmostFinalValueBenchSettings {
+  implicit class SettingsOps(private val settings: AlmostFinalValueBenchSettings) extends AnyVal {
+    @inline final def isTrue3: Boolean = AlmostFinalValueBenchmarkStatics.isTrue && settings.flag
+  }
+
+  @inline def isTrue4(settings: AlmostFinalValueBenchSettings): Boolean =
+    AlmostFinalValueBenchmarkStatics.isTrue && settings.flag
+}
+
+@Warmup(iterations = 10, time = 1, timeUnit = TimeUnit.SECONDS)
+@Measurement(iterations = 10, time = 1, timeUnit = TimeUnit.SECONDS)
+@Fork(3)
+@BenchmarkMode(Array(Mode.AverageTime))
+@OutputTimeUnit(TimeUnit.NANOSECONDS)
+@State(Scope.Benchmark)
+class AlmostFinalValueBenchmark {
+  import AlmostFinalValueBenchmarkStatics.STATIC_FINAL_FALSE
+  val settings = new AlmostFinalValueBenchSettings(); import settings._
+
+  private def pretendToWorkHard() = Blackhole.consumeCPU(3)
+
+  @Benchmark def bench0_unit                  = ()
+  @Benchmark def bench0_usingStaticFinalFalse = if (STATIC_FINAL_FALSE && flag) pretendToWorkHard()
+  @Benchmark def bench0_workingHard           = pretendToWorkHard()
+
+  @Benchmark def bench1_usingAlmostFinalFalse = if (AlmostFinalValueBenchmarkStatics.isTrue && flag) pretendToWorkHard()
+  @Benchmark def bench2_usingInlineMethod     = if (settings.isTrue2) pretendToWorkHard()
+  @Benchmark def bench3_usingExtMethod        = if (settings.isTrue3) pretendToWorkHard()
+  @Benchmark def bench4_usingObjectMethod     = if (AlmostFinalValueBenchSettings.isTrue4(settings)) pretendToWorkHard()
+
+/*
+  This benchmark is measuring two things:
+  1. verifying that using AlmostFinalValue in an if block makes the block a no-op
+  2. verifying and comparing which ergonomic wrapper around AlmostFinalValue maintains that
+
+  The first point is satisfied.
+
+  For the second:
+  1. inline instance methods add a null-check overhead, slowing it down
+  2. extension methods perform as quickly, are very ergonomic and so are the best choice
+  3. object methods also perform as quickly, but can be less ergonomic if it requires an import
+*/
+}