Reduce ops when setting exisitng execution env (#9967)

Setting execution environment to the existing one should have no effect. Should (positively) affect startup in #9789. # Important Notes Cancelling jobs and triggering a fresh execute job is expensive and unnecessary, especially on startup, when the result should be the same as before.
enso-org · May 17, 2024 · 1c4eb6e · 1c4eb6e
1 parent 96476f6
commit 1c4eb6e
Show file tree

Hide file tree

Showing 6 changed files with 57 additions and 17 deletions.
diff --git a/build.sbt b/build.sbt
@@ -742,14 +742,16 @@ lazy val `profiling-utils` = project
       exclude ("org.netbeans.api", "org-openide-awt")
       exclude ("org.netbeans.api", "org-openide-modules")
       exclude ("org.netbeans.api", "org-netbeans-api-annotations-common"),
+      "org.slf4j"      % "slf4j-api"       % slf4jVersion,
       "junit"          % "junit"           % junitVersion   % Test,
       "com.github.sbt" % "junit-interface" % junitIfVersion % Test
     ),
     modulePath := {
       JPMSUtils.filterModulesFromUpdate(
         update.value,
         Seq(
-          "org.netbeans.api" % "org-netbeans-modules-sampler" % netbeansApiVersion
+          "org.netbeans.api" % "org-netbeans-modules-sampler" % netbeansApiVersion,
+          "org.slf4j"        % "slf4j-api"                    % slf4jVersion
         ),
         streams.value.log,
         shouldContainAll = true

diff --git a/...src/main/java/org/enso/interpreter/instrument/command/SetExecutionEnvironmentCommand.java b/...src/main/java/org/enso/interpreter/instrument/command/SetExecutionEnvironmentCommand.java
@@ -42,25 +42,34 @@ public Future<BoxedUnit> executeAsynchronously(RuntimeContext ctx, ExecutionCont
   private void setExecutionEnvironment(
       Runtime$Api$ExecutionEnvironment executionEnvironment, UUID contextId, RuntimeContext ctx) {
     var logger = ctx.executionService().getLogger();
-    ctx.jobControlPlane().abortJobs(contextId);
     var contextLockTimestamp = ctx.locking().acquireContextLock(contextId);
+
     try {
-      var writeLockTimestamp = ctx.locking().acquireWriteCompilationLock();
-      try {
-        Stack<InstrumentFrame> stack = ctx.contextManager().getStack(contextId);
-        ctx.executionService()
-            .getContext()
-            .setExecutionEnvironment(ExecutionEnvironment.forName(executionEnvironment.name()));
-        CacheInvalidation.invalidateAll(stack);
-        ctx.jobProcessor().run(ExecuteJob.apply(contextId, stack.toList()));
-        reply(new Runtime$Api$SetExecutionEnvironmentResponse(contextId), ctx);
-      } finally {
-        ctx.locking().releaseWriteCompilationLock();
+      var oldEnvironment = ctx.executionService().getContext().getExecutionEnvironment();
+      if (!oldEnvironment.getName().equals(executionEnvironment.name())) {
+        ctx.jobControlPlane().abortJobs(contextId);
+        var writeLockTimestamp = ctx.locking().acquireWriteCompilationLock();
+        try {
+          Stack<InstrumentFrame> stack = ctx.contextManager().getStack(contextId);
+          ctx.executionService()
+              .getContext()
+              .setExecutionEnvironment(ExecutionEnvironment.forName(executionEnvironment.name()));
+          CacheInvalidation.invalidateAll(stack);
+          ctx.jobProcessor().run(ExecuteJob.apply(contextId, stack.toList()));
+          reply(new Runtime$Api$SetExecutionEnvironmentResponse(contextId), ctx);
+        } finally {
+          ctx.locking().releaseWriteCompilationLock();
+          logger.log(
+              Level.FINEST,
+              "Kept write compilation lock [SetExecutionEnvironmentCommand] for "
+                  + (System.currentTimeMillis() - writeLockTimestamp)
+                  + " milliseconds");
+        }
+      } else {
         logger.log(
             Level.FINEST,
-            "Kept write compilation lock [SetExecutionEnvironmentCommand] for "
-                + (System.currentTimeMillis() - writeLockTimestamp)
-                + " milliseconds");
+            "Requested environment is the same as the current one. Request has no effect");
+        reply(new Runtime$Api$SetExecutionEnvironmentResponse(contextId), ctx);
       }
     } finally {
       ctx.locking().releaseContextLock(contextId);

diff --git a/...src/test/scala/org/enso/interpreter/test/instrument/RuntimeExecutionEnvironmentTest.scala b/...src/test/scala/org/enso/interpreter/test/instrument/RuntimeExecutionEnvironmentTest.scala
@@ -304,6 +304,24 @@ class RuntimeExecutionEnvironmentTest
       .Design()
       .name
 
+    // setting execution environment to the existing one has no effect
+    context.send(
+      Api.Request(
+        requestId,
+        Api.SetExecutionEnvironmentRequest(
+          contextId,
+          Api.ExecutionEnvironment.Design()
+        )
+      )
+    )
+
+    context.receiveNIgnoreStdLib(1) should contain theSameElementsAs Seq(
+      Api.Response(requestId, Api.SetExecutionEnvironmentResponse(contextId))
+    )
+    context.languageContext.getExecutionEnvironment.getName shouldEqual Api.ExecutionEnvironment
+      .Design()
+      .name
+
     // set execution environment
     context.send(
       Api.Request(

diff --git a/lib/scala/profiling-utils/src/main/java/module-info.java b/lib/scala/profiling-utils/src/main/java/module-info.java
@@ -1,6 +1,7 @@
 module org.enso.profiling {
   requires org.netbeans.modules.sampler.RELEASE180;
   requires jdk.management;
+  requires org.slf4j;
 
   exports org.enso.profiling.sampler;
   exports org.enso.profiling.events;

diff --git a/lib/scala/profiling-utils/src/main/java/org/enso/profiling/sampler/OutputStreamSampler.java b/lib/scala/profiling-utils/src/main/java/org/enso/profiling/sampler/OutputStreamSampler.java
@@ -7,6 +7,8 @@
 import java.io.IOException;
 import java.io.OutputStream;
 import org.netbeans.modules.sampler.Sampler;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Gathers application performance statistics that can be visualised in Java VisualVM, and writes it
@@ -19,6 +21,8 @@ public final class OutputStreamSampler implements MethodsSampler {
 
   private boolean isSamplingStarted = false;
 
+  private static Logger logger = LoggerFactory.getLogger(OutputStreamSampler.class);
+
   /**
    * Creates the {@link OutputStreamSampler} for provided output stream.
    *
@@ -36,6 +40,7 @@ public static OutputStreamSampler ofFile(File file) throws FileNotFoundException
   public void start() {
     synchronized (this) {
       if (sampler != null && !isSamplingStarted) {
+        logger.trace("Starting profiling sampler");
         sampler.start();
         isSamplingStarted = true;
       }
@@ -46,6 +51,7 @@ public void start() {
   public void stop() throws IOException {
     synchronized (this) {
       if (isSamplingStarted) {
+        logger.trace("Stopping profiling sampler");
         try (DataOutputStream dos = new DataOutputStream(outputStream)) {
           sampler.stopAndWriteTo(dos);
         }

diff --git a/...runtime-version-manager/src/main/scala/org/enso/runtimeversionmanager/runner/Runner.scala b/...runtime-version-manager/src/main/scala/org/enso/runtimeversionmanager/runner/Runner.scala
@@ -222,7 +222,11 @@ class Runner(
         else Seq()
 
       val command = Seq(javaCommand.executableName) ++
-        jvmArguments ++ loggingConnectionArguments ++ runSettings.runnerArguments
+        Seq(
+          "-XX:ActiveProcessorCount=2",
+          "-Xmx8192m"
+          //"-agentlib:jdwp=transport=dt_socket,server=y,address=localhost:5005,suspend=y"
+        ) ++ jvmArguments ++ loggingConnectionArguments ++ runSettings.runnerArguments
 
       val distributionSettings =
         distributionManager.getEnvironmentToInheritSettings