Disable visualizations for subexpressions

The possibility of attaching visualizations to subexpressions meant that we (currently) have to invalidate caches for their parent expression every time a request comes. That was an acceptable cost when an expression is relatively fast to compute but unacceptable when dealing with slow computations that would have to be repeated. Since currently attaching visualizations is not used at all and we can rely on caching RHS and self, it is _safe_ to disable it. An observable pattern is better suited for visualizations and would mitigate this problem by design.
enso-org · Dec 30, 2024 · 2b35f97 · 2b35f97
1 parent 10d95f1
commit 2b35f97
Show file tree

Hide file tree

Showing 5 changed files with 12 additions and 5 deletions.
diff --git a/...-common/src/main/scala/org/enso/interpreter/instrument/execution/JobExecutionEngine.scala b/...-common/src/main/scala/org/enso/interpreter/instrument/execution/JobExecutionEngine.scala
@@ -96,6 +96,7 @@ final class JobExecutionEngine(
             assertInJvm(timeSinceRequestedToCancel > 0)
             val timeToCancel =
               forceInterruptTimeout - timeSinceRequestedToCancel
+            assertInJvm(timeToCancel > 0)
             logger.log(
               Level.FINEST,
               "About to wait {}ms  to cancel job {}",
@@ -107,7 +108,8 @@ final class JobExecutionEngine(
             runningJob.future.get(timeToCancel, TimeUnit.MILLISECONDS)
             logger.log(
               Level.FINEST,
-              "Job {} finished within the allocated soft-cancel time"
+              "Job {} finished within the allocated soft-cancel time",
+              runningJob.id
             )
           } catch {
             case _: TimeoutException =>

diff --git a/...trument-common/src/main/scala/org/enso/interpreter/instrument/job/EnsureCompiledJob.scala b/...trument-common/src/main/scala/org/enso/interpreter/instrument/job/EnsureCompiledJob.scala
@@ -484,6 +484,8 @@ class EnsureCompiledJob(
       )
     invalidatedVisualizations.foreach { visualization =>
       UpsertVisualizationJob.upsertVisualization(visualization)
+    // Cache invalidation disabled because of #11882
+    // ctx.state.executionHooks.add(InvalidateCaches(visualization.expressionId))
     }
     if (invalidatedVisualizations.nonEmpty) {
       ctx.executionService.getLogger.log(

diff --git a/...t-common/src/main/scala/org/enso/interpreter/instrument/job/ProgramExecutionSupport.scala b/...t-common/src/main/scala/org/enso/interpreter/instrument/job/ProgramExecutionSupport.scala
@@ -94,9 +94,9 @@ object ProgramExecutionSupport {
 
     val onComputedValueCallback: Consumer[ExpressionValue] = { value =>
       if (callStack.isEmpty) {
-        logger.log(Level.FINEST, s"ON_COMPUTED ${value.getExpressionId}")
 
         if (VisualizationResult.isInterruptedException(value.getValue)) {
+          logger.log(Level.FINEST, s"ON_INTERRUPTED ${value.getExpressionId}")
           value.getValue match {
             case e: AbstractTruffleException =>
               sendInterruptedExpressionUpdate(
@@ -110,6 +110,7 @@ object ProgramExecutionSupport {
             case _ =>
           }
         }
+        logger.log(Level.FINEST, s"ON_COMPUTED ${value.getExpressionId}")
         sendExpressionUpdate(contextId, executionFrame.syncState, value)
         sendVisualizationUpdates(
           contextId,

diff --git a/...nt-common/src/main/scala/org/enso/interpreter/instrument/job/UpsertVisualizationJob.scala b/...nt-common/src/main/scala/org/enso/interpreter/instrument/job/UpsertVisualizationJob.scala
@@ -121,6 +121,8 @@ class UpsertVisualizationJob(
                 )
                 None
               case None =>
+                // Caching disabled due to #11882
+                // ctx.state.executionHooks.add(InvalidateCaches(expressionId))
                 Some(Executable(config.executionContextId, stack))
             }
         }
@@ -160,7 +162,7 @@ class UpsertVisualizationJob(
 object UpsertVisualizationJob {
 
   /** Invalidate caches for a particular expression id. */
-  sealed private case class InvalidateCaches(
+  sealed case class InvalidateCaches(
     expressionId: Api.ExpressionId
   )(implicit ctx: RuntimeContext)
       extends Runnable {
@@ -511,7 +513,6 @@ object UpsertVisualizationJob {
         arguments
       )
     setCacheWeights(visualization)
-    ctx.state.executionHooks.add(InvalidateCaches(expressionId))
     ctx.contextManager.upsertVisualization(
       visualizationConfig.executionContextId,
       visualization

diff --git a/...tests/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualizationsTest.scala b/...tests/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualizationsTest.scala
@@ -4419,7 +4419,8 @@ class RuntimeVisualizationsTest extends AnyFlatSpec with Matchers {
       new String(data1, StandardCharsets.UTF_8) shouldEqual "C"
   }
 
-  it should "emit visualization update for the target of a method call (subexpression) with IdMap" in withContext() {
+  // Attaching visualizations to subexpressions is currently disabled, see #11882
+  ignore should "emit visualization update for the target of a method call (subexpression) with IdMap" in withContext() {
     context =>
       val contextId       = UUID.randomUUID()
       val requestId       = UUID.randomUUID()