Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Further rework State manipulation Nodes #12294

Open
JaroslavTulach opened this issue Feb 16, 2025 · 3 comments · May be fixed by #12314
Open

Further rework State manipulation Nodes #12294

JaroslavTulach opened this issue Feb 16, 2025 · 3 comments · May be fixed by #12314
Assignees

Comments

@JaroslavTulach
Copy link
Member

JaroslavTulach commented Feb 16, 2025

Originally posted by @JaroslavTulach in #12233 (comment) reporting benchmarks slowdown for

RecursionBenchmarks_benchSumStateTCO
RecursionBenchmarks_benchNestedThunkSum

this slowdown is not fully deterministic. When using IGV it may disappear - e.g. it probably depends on length of JVM profiling or something like that.

Anyway, the State manipulation deserves further changes:

  • ExecutionEnvironment should be merged into the State (CCing @4e6) - Remove Runtime.current_execution_environment & co. #12340
  • Move - State update manipulation this nodes is into the same package
  • benchSumStateTCO andbenchNestedThunkSum benchmarks.
  • rework hide RecursionBenchmarks access to use State.Controller SrcUtil (CCing @Akirathan)
    while doing this, also more deeply evaluate the benchSumStateTCO andbenchNestedThunkSum benchmarks.
  • rework RecursionBenchmarks to use SrcUtil (CCing @Akirathan)
@enso-bot
Copy link

enso-bot bot commented Feb 19, 2025

Jaroslav Tulach reports a new STANDUP for yesterday (2025-02-18):

Progress: .

@enso-bot
Copy link

enso-bot bot commented Feb 20, 2025

Jaroslav Tulach reports a new STANDUP for yesterday (2025-02-19):

Progress: .

  • Investigating RunStateNode test failures: f5cb289
  • Writing RuntimeServerTesting: 5f0c43f
  • removing @Builtin_Methods: d379995
  • IGVing with Radek
  • many meetings It should be finished by 2025-02-25.

@JaroslavTulach
Copy link
Member Author

This is the change I tried and decided that such a rework is too complicated for #12314

diff --git engine/runtime/src/main/java/org/enso/interpreter/EnsoLanguage.java engine/runtime/src/main/java/org/enso/interpreter/EnsoLanguage.java
index 0968178d96..7b8f9e9df8 100644
--- engine/runtime/src/main/java/org/enso/interpreter/EnsoLanguage.java
+++ engine/runtime/src/main/java/org/enso/interpreter/EnsoLanguage.java
@@ -111,8 +111,6 @@ public final class EnsoLanguage extends TruffleLanguage<EnsoContext> {
   private static final LanguageReference<EnsoLanguage> REFERENCE =
       LanguageReference.create(EnsoLanguage.class);
 
-  private final ContextThreadLocal<ExecutionEnvironment[]> executionEnvironment =
-      locals.createContextThreadLocal((ctx, thread) -> new ExecutionEnvironment[1]);
   private final ContextThreadLocal<State> state =
       locals.createContextThreadLocal((ctx, thread) -> State.create(ctx));
 
@@ -472,14 +470,6 @@ public final class EnsoLanguage extends TruffleLanguage<EnsoContext> {
     return null;
   }
 
-  public ExecutionEnvironment getExecutionEnvironment() {
-    return executionEnvironment.get()[0];
-  }
-
-  public void setExecutionEnvironment(ExecutionEnvironment executionEnvironment) {
-    this.executionEnvironment.get()[0] = executionEnvironment;
-  }
-
   /** Access to state associated with current context and thread. */
   public final State currentState() {
     return this.state.get();
* Unmerged path engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/runtime/ContextIsEnabledNode.java
diff --git engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/runtime/RuntimeWithDisabledContextNode.java engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/runtime/RuntimeWithDisabledContextNode.java
deleted file mode 100644
index f03734ba69..0000000000
--- engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/runtime/RuntimeWithDisabledContextNode.java
+++ /dev/null
@@ -1,36 +0,0 @@
-package org.enso.interpreter.node.expression.builtin.runtime;
-
-import com.oracle.truffle.api.frame.VirtualFrame;
-import com.oracle.truffle.api.nodes.Node;
-import org.enso.interpreter.dsl.BuiltinMethod;
-import org.enso.interpreter.dsl.Suspend;
-import org.enso.interpreter.node.BaseNode;
-import org.enso.interpreter.node.callable.thunk.ThunkExecutorNode;
-import org.enso.interpreter.node.expression.builtin.text.util.ExpectStringNode;
-import org.enso.interpreter.runtime.EnsoContext;
-import org.enso.interpreter.runtime.data.atom.Atom;
-import org.enso.interpreter.runtime.state.ExecutionEnvironment;
-
-@BuiltinMethod(
-    type = "Runtime",
-    name = "with_disabled_context_builtin",
-    description = "Disallows context in the specified scope.",
-    autoRegister = false,
-    inlineable = true)
-public class RuntimeWithDisabledContextNode extends Node {
-  private @Child ThunkExecutorNode thunkExecutorNode = ThunkExecutorNode.build();
-  private @Child ExpectStringNode expectStringNode = ExpectStringNode.build();
-
-  Object execute(VirtualFrame frame, Atom context, Object env_name, @Suspend Object action) {
-    var ctx = EnsoContext.get(this);
-    var state = ctx.currentState();
-    String envName = expectStringNode.execute(env_name);
-    ExecutionEnvironment original =
-        EnsoContext.get(this).disableExecutionEnvironment(context, envName);
-    try {
-      return thunkExecutorNode.executeThunk(frame, action, state, BaseNode.TailStatus.NOT_TAIL);
-    } finally {
-      EnsoContext.get(this).setExecutionEnvironment(original);
-    }
-  }
-}
diff --git engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/runtime/RuntimeWithEnabledContextNode.java engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/runtime/RuntimeWithEnabledContextNode.java
deleted file mode 100644
index c2be773e4d..0000000000
--- engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/runtime/RuntimeWithEnabledContextNode.java
+++ /dev/null
@@ -1,36 +0,0 @@
-package org.enso.interpreter.node.expression.builtin.runtime;
-
-import com.oracle.truffle.api.frame.VirtualFrame;
-import com.oracle.truffle.api.nodes.Node;
-import org.enso.interpreter.dsl.BuiltinMethod;
-import org.enso.interpreter.dsl.Suspend;
-import org.enso.interpreter.node.BaseNode;
-import org.enso.interpreter.node.callable.thunk.ThunkExecutorNode;
-import org.enso.interpreter.node.expression.builtin.text.util.ExpectStringNode;
-import org.enso.interpreter.runtime.EnsoContext;
-import org.enso.interpreter.runtime.data.atom.Atom;
-import org.enso.interpreter.runtime.state.ExecutionEnvironment;
-
-@BuiltinMethod(
-    type = "Runtime",
-    name = "with_enabled_context_builtin",
-    description = "Allows context in the specified scope.",
-    autoRegister = false,
-    inlineable = true)
-public class RuntimeWithEnabledContextNode extends Node {
-  private @Child ThunkExecutorNode thunkExecutorNode = ThunkExecutorNode.build();
-  private @Child ExpectStringNode expectStringNode = ExpectStringNode.build();
-
-  Object execute(VirtualFrame frame, Atom context, Object env_name, @Suspend Object action) {
-    var ctx = EnsoContext.get(this);
-    var state = ctx.currentState();
-    String envName = expectStringNode.execute(env_name);
-    ExecutionEnvironment original =
-        EnsoContext.get(this).enableExecutionEnvironment(context, envName);
-    try {
-      return thunkExecutorNode.executeThunk(frame, action, state, BaseNode.TailStatus.NOT_TAIL);
-    } finally {
-      EnsoContext.get(this).setExecutionEnvironment(original);
-    }
-  }
-}
diff --git engine/runtime/src/main/java/org/enso/interpreter/runtime/EnsoContext.java engine/runtime/src/main/java/org/enso/interpreter/runtime/EnsoContext.java
index 5ecc5db507..6c8c818e78 100644
--- engine/runtime/src/main/java/org/enso/interpreter/runtime/EnsoContext.java
+++ engine/runtime/src/main/java/org/enso/interpreter/runtime/EnsoContext.java
@@ -873,29 +873,29 @@ public final class EnsoContext {
   public long nextSequenceId() {
     return clock.getAndIncrement();
   }
-
+/*
   public ExecutionEnvironment getGlobalExecutionEnvironment() {
     return globalExecutionEnvironment;
   }
-
+  */
   public ExecutionEnvironment getExecutionEnvironment() {
-    ExecutionEnvironment env = language.getExecutionEnvironment();
-    return env == null ? getGlobalExecutionEnvironment() : env;
+    ExecutionEnvironment env = null; // XXX language.getExecutionEnvironment();
+    return env == null ? globalExecutionEnvironment : env;
   }
+  /*
 
-  /** Set the runtime execution environment of this context. */
+  /** Set the runtime execution environment of this context. *
   public void setExecutionEnvironment(ExecutionEnvironment executionEnvironment) {
     this.globalExecutionEnvironment = executionEnvironment;
-    language.setExecutionEnvironment(executionEnvironment);
+    // language.setExecutionEnvironment(executionEnvironment);
   }
-
+*/
   /**
    * Enable execution context in the execution environment.
    *
    * @param context the execution context
    * @param environmentName the execution environment name
    * @return the execution environment version before modification
-   */
   public ExecutionEnvironment enableExecutionEnvironment(Atom context, String environmentName) {
     ExecutionEnvironment original = globalExecutionEnvironment;
     if (original.getName().equals(environmentName)) {
@@ -905,6 +905,7 @@ public final class EnsoContext {
     }
     return original;
   }
+   */
 
   /**
    * Enable execution context in the execution environment.
@@ -912,7 +913,6 @@ public final class EnsoContext {
    * @param context the execution context
    * @param environmentName the execution environment name
    * @return the execution environment version before modification
-   */
   public ExecutionEnvironment disableExecutionEnvironment(Atom context, String environmentName) {
     ExecutionEnvironment original = globalExecutionEnvironment;
     if (original.getName().equals(environmentName)) {
@@ -922,6 +922,7 @@ public final class EnsoContext {
     }
     return original;
   }
+   */
 
   /** Returns a maximal number of warnings that can be attached to a value */
   public int getWarningsLimit() {
diff --git engine/runtime/src/main/java/org/enso/interpreter/runtime/state/ExecutionEnvironment.java engine/runtime/src/main/java/org/enso/interpreter/runtime/state/ExecutionEnvironment.java
index 86421b3aa6..21385b24ff 100644
--- engine/runtime/src/main/java/org/enso/interpreter/runtime/state/ExecutionEnvironment.java
+++ engine/runtime/src/main/java/org/enso/interpreter/runtime/state/ExecutionEnvironment.java
@@ -1,46 +1,55 @@
 package org.enso.interpreter.runtime.state;
 
-public class ExecutionEnvironment {
+/** Representation of an execution environment. Currently Either */
+public final class ExecutionEnvironment {
+  private static final String LIVE_NAME = "live";
+  private static final String DESIGN_NAME = "design";
+  public static final ExecutionEnvironment LIVE;
+  static {
+    var permissions = new ContextPermissions(true, true, false);
+    LIVE = new ExecutionEnvironment(LIVE_NAME, permissions);
+  }
+  public static final ExecutionEnvironment DESIGN = new ExecutionEnvironment(DESIGN_NAME);
 
   private final String name;
-
   final ContextPermissions permissions;
 
-  public static final String LIVE_ENVIRONMENT_NAME = "live";
-
-  public static final String DESIGN_ENVIRONMENT_NAME = "design";
-
-  public static final ExecutionEnvironment LIVE = initLive(LIVE_ENVIRONMENT_NAME);
-  public static final ExecutionEnvironment DESIGN =
-      new ExecutionEnvironment(DESIGN_ENVIRONMENT_NAME);
-
-  private static ExecutionEnvironment initLive(String name) {
-    var permissions = new ContextPermissions(true, true, false);
-    return new ExecutionEnvironment(name, permissions);
-  }
-
-  public ExecutionEnvironment(String name) {
+  private ExecutionEnvironment(String name) {
     this.name = name;
     this.permissions = new ContextPermissions(false, false, false);
   }
 
-  ExecutionEnvironment(String name, ContextPermissions permissions) {
+  private ExecutionEnvironment(String name, ContextPermissions permissions) {
     this.name = name;
     this.permissions = permissions;
   }
 
+  /**
+   * The name of this environment.
+   *
+   * @return
+   */
   public String getName() {
     return this.name;
   }
 
+  /**
+   * Creates an {@link ExecutionEnvironment} for provided name.
+   *
+   * @param name either {@code "live"} or {@code "design"}
+   * @return either {@link #LIVE} or {@link #DESIGN}
+   * @throws IllegalArgumentException if the argument isn't recognized
+   */
   public static ExecutionEnvironment forName(String name) {
-    switch (name) {
-      case LIVE_ENVIRONMENT_NAME:
-        return LIVE;
-      case DESIGN_ENVIRONMENT_NAME:
-        return DESIGN;
-      default:
-        throw new IllegalArgumentException("Unsupported Execution Environment `" + name + "`");
-    }
+    return switch (name) {
+      case LIVE_NAME -> LIVE;
+      case DESIGN_NAME -> DESIGN;
+      default -> throw new IllegalArgumentException(
+          "Unsupported Execution Environment `" + name + "`");
+    };
+  }
+
+  final ExecutionEnvironment copyWithPermissions(ContextPermissions newPermissions) {
+    return new ExecutionEnvironment(this.name, newPermissions);
   }
 }
diff --git engine/runtime/src/main/java/org/enso/interpreter/runtime/state/WithContextNode.java engine/runtime/src/main/java/org/enso/interpreter/runtime/state/WithContextNode.java
index 2c7140d608..00fdcb12a1 100644
--- engine/runtime/src/main/java/org/enso/interpreter/runtime/state/WithContextNode.java
+++ engine/runtime/src/main/java/org/enso/interpreter/runtime/state/WithContextNode.java
@@ -55,6 +55,6 @@ public abstract class WithContextNode extends Node {
     } else {
       throw ensoCtx.raiseAssertionPanic(this, "Unknown context: " + ctor, null);
     }
-    return new ExecutionEnvironment(current.getName(), newPermissions);
+    return current.copyWithPermissions(newPermissions);
   }
 }
diff --git test/micro-distribution/lib/Standard/Base/0.0.0-dev/src/Data/Numbers.enso test/micro-distribution/lib/Standard/Base/0.0.0-dev/src/Data/Numbers.enso
index faa4d3a26c..6eaa6f3be9 100644
--- test/micro-distribution/lib/Standard/Base/0.0.0-dev/src/Data/Numbers.enso
+++ test/micro-distribution/lib/Standard/Base/0.0.0-dev/src/Data/Numbers.enso
@@ -17,6 +17,10 @@ type Integer
     > self that = Integer.do_greater self that
     >= self that = Integer.do_greater_equal self that
 
+    bit_and self that = Integer.do_bit_and self that
+    bit_or self that = Integer.do_bit_or self that
+    bit_not self = Integer.do_bit_not self
+
 
 
     do_pow a b = @Builtin_Method "Integer.^"
@@ -32,3 +36,7 @@ type Integer
     do_less_equal a b = @Builtin_Method "Integer.<="
     do_greater a b = @Builtin_Method "Integer.>"
     do_greater_equal a b = @Builtin_Method "Integer.>="
+
+    do_bit_and a b = @Builtin_Method "Integer.bit_and"
+    do_bit_or a b = @Builtin_Method "Integer.bit_or"
+    do_bit_not a = @Builtin_Method "Integer.bit_not"
diff --git test/micro-distribution/lib/Standard/Base/0.0.0-dev/src/Runtime.enso test/micro-distribution/lib/Standard/Base/0.0.0-dev/src/Runtime.enso
index b4d93ebc23..39383b2f94 100644
--- test/micro-distribution/lib/Standard/Base/0.0.0-dev/src/Runtime.enso
+++ test/micro-distribution/lib/Standard/Base/0.0.0-dev/src/Runtime.enso
@@ -1,6 +1,7 @@
 import project.Any.Any
 import project.Data.Text.Text
 import project.Data.Boolean.Boolean
+import project.Data.Numbers.Integer
 import project.Error.Error
 import project.Errors.Common.Forbidden_Operation
 import project.Function.Function
@@ -9,6 +10,7 @@ import project.Panic.Panic
 import project.Data.Boolean.Boolean.True
 import project.Runtime.Context.Input
 import project.Runtime.Context.Output
+import project.Runtime.State
 
 @Builtin_Type
 type Context
@@ -30,26 +32,40 @@ type Context
 
     is_enabled : Text -> Boolean
     is_enabled self environment=Runtime.current_execution_environment =
-        is_enabled_builtin self environment
+        Context_Helper.is_enabled self environment
 
-## PRIVATE
-is_enabled_builtin : Text -> Boolean
-is_enabled_builtin c environment = @Builtin_Method "Context.is_enabled_builtin"
+type Context_Helper
+    private to_mask c:Context = case c of
+        Context.Input -> 1
+        Context.Output -> 2
+        Context.Dataflow_Stack_Trace -> 3
+        _ -> Error.throw "Unknown context "+c.to_text
+
+    is_enabled c:Context env:Text =
+        if env != Runtime.current_execution_environment then Panic.throw "Unexpected env" else
+            mask = Context_Helper.to_mask c
+            mask.bit_and current_mask != 0
+
+private current_mask -> Integer =
+    Panic.catch Any (State.get Context_Helper) (_->0)
 
 current_execution_environment : Text
 current_execution_environment = @Builtin_Method "Runtime.current_execution_environment"
 
-with_enabled_context : Context -> Text -> Function -> Any
-with_enabled_context context environment=Runtime.current_execution_environment ~action = with_enabled_context_builtin context environment action
+with_enabled_context context environment=Runtime.current_execution_environment ~action =
+    if environment != Runtime.current_execution_environment then action else
+        plus_mask = Context_Helper.to_mask context
+        mix_mask = current_mask.bit_or plus_mask : Integer
+        State.run Context_Helper mix_mask action...
 
-with_enabled_context_builtin : Context -> Text -> Function -> Any
-with_enabled_context_builtin context environment ~action = @Builtin_Method "Runtime.with_enabled_context_builtin"
 
 with_disabled_context : Context -> Text -> Function -> Any
-with_disabled_context context environment=Runtime.current_execution_environment ~action = with_disabled_context_builtin context environment action
-
-with_disabled_context_builtin : Context -> Text -> Function -> Any
-with_disabled_context_builtin context environment ~action = @Builtin_Method "Runtime.with_disabled_context_builtin"
+with_disabled_context context environment=Runtime.current_execution_environment ~action =
+    if environment != Runtime.current_execution_environment then action else
+        current_mask = State.get Context_Helper : Integer
+        keep_mask = Context_Helper.to_mask context . bit_not
+        mix_mask = current_mask.bit_and keep_mask : Integer
+        State.run Context_Helper mix_mask action...
 
 primitive_get_stack_trace = @Builtin_Method "Runtime.primitive_get_stack_trace"
 

Please note the following line in the reimplementation of current behavior (in Java builtin) in Enso:

with_disabled_context context environment=Runtime.current_execution_environment ~action =
   if environment != Runtime.current_execution_environment then action else

I find it very suspicious...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🔧 Implementation
Development

Successfully merging a pull request may close this issue.

1 participant