Binding is slow

[JonHenson]

(NOTE: A repository with code that demonstrates the issue is here: js-engine-performance)

I'm evaluating the feasibility of moving from using Rhino to GraalJS. The scenario is user defined expressions are being evaluated in a Context that has a number of variables bound to it, each expression may or may not refer to some of the bound variables. A Context may be short-lived, so it's important that creating them and binding variables is speedy - unfortunately both creating a polyglot context and binding variables in it is significantly slower than using Rhino.

I believe the issue with slow Context creation can be alleviated by using a pool of Contexts rather then creating a new one each time. The code linked above is somewhat simpler, as it's only running single-threaded and so can simply clear out the bindings and reuse a single Context.

The bindings problem is a thornier issue. The linked code times how long it takes to do 30,000: context creation; variable bind (100 variables); expression evaluation; context close.
On my machine this takes Rhino approximately 0.2 seconds, but GraalJS takes around 20 seconds.

In the case where an expression on references a small number of variables, performance can be improved by pre-parsing the expression to be evaluated and only binding referenced variables (the example code does very basic parsing and assumes each variable is surrounded by white space). Rather than having to parse expressions before evaluation, is there a way in which a variable can be bound at the point the engine tries to access a value and finds it's not bound? Better still, is there any way to make binding faster?

I'm running using:

java version "21.0.1" 2023-10-17
Java(TM) SE Runtime Environment Oracle GraalVM 21.0.1+12.1 (build 21.0.1+12-jvmci-23.1-b19)
Java HotSpot(TM) 64-Bit Server VM Oracle GraalVM 21.0.1+12.1 (build 21.0.1+12-jvmci-23.1-b19, mixed mode, sharing)

With version 23.1.1 of the Graal libraries.

Full code is in the linke repo, but as an outline:

I have an Evaluator interface, the GraalJs implementation is created using the following factory:

  private static Supplier<Evaluator> graalEvaluatorFactory(Function<String, Source> sourceFactory) {
    Context.Builder ctxBuilder = Context.newBuilder("js").engine(Engine.create("js"));
    // Use the same Context for every Evaluator
    Context ctx = ctxBuilder.build();
    Value bindings = ctx.getBindings("js");
    return () -> {
      return new Evaluator() {
        @Override
        public void close() {
          // Creating a context is slow, so simply clear out the bindings so it can be
          // reused.
          // NOTE: This is only OK because we're running in a single thread.
          //
          // NOTE: bindings.getMemberKeys().clear() throws an exception, as does
          // using an Iterator and calling remove().
          bindings.getMemberKeys().forEach(s -> bindings.removeMember(s));
        }

        @Override
        public Object eval(String snippet) {
          return ctx.eval(sourceFactory.apply(snippet)).as(Object.class);
        }

        @Override
        public void bind(String name, Object value) {
          bindings.putMember(name, value);
        }
      };
    };
  }

The sourceFatory is one of these (performance of both is similar):

  private static final Function<String, Source> nonCachingSourceFactory() {
    return snippet -> Source.create("js", snippet);
  }

  private static final Function<String, Source> cachingSourceFactory() {
    Map<String, Source> sources = new HashMap<>();
    return snippet -> sources.computeIfAbsent(snippet, s -> {
      System.out.println("Caching: " + s);
      return Source.create("js", s);
    });
  }

With a "binder" to bind values defined in a variables map:

    Map<String, Object> variables = variables();

    // Bind all variables in the Map into an Evaluator
    BiConsumer<Evaluator, String> bindAll =
        (evaluator, snippet) -> variables.forEach((name, value) -> evaluator.bind(name, value));
    // Iterate over extracted tokens and bind if they have a value in the variables
    // map
    BiConsumer<Evaluator, String> bindReferenced = (evaluator, snippet) -> {
      tokensFromSnippet(snippet).forEach(token -> {
        if (variables.containsKey(token)) {
          evaluator.bind(token, variables.get(token));
        }
      });
    };

With a single evaluation being performed thus:

  private static Object eval(Supplier<Evaluator> evaluatorFactory,
      BiConsumer<Evaluator, String> binder, String snippet) {
    try (Evaluator evaluator = evaluatorFactory.get()) {
      binder.accept(evaluator, snippet);
      return evaluator.eval(snippet);
    }
  } 

[iamstolis]

You are right that putMember() and removeMember() are not overly fast. The root of the problem is that they are generic (work for any property/member) and as such cannot specialize itself well. I suggest you to use evaluation of an appropriate script instead. A script that contains a separate assignment for each binding can specialize each assignment and can be much faster. You can imagine that each assignment remembers where to write the value while the general method has to find the right place with each invocation.

So, you can rewrite your bindAll to something like

StringBuilder sb = new StringBuilder();
variables.forEach((name, value) -> {
    sb.append(name).append("=variables['").append(name).append("'];");
});
String script = sb.toString();
BiConsumer<Evaluator, String> bindAll =
    (evaluator, snippet) -> {
        evaluator.bind("variables", ProxyObject.fromMap(variables));
        evaluator.eval(script);
    };

There is a similar problem with the repeated invocation of removeMember() in close(). You can rewrite it to

StringBuilder sb = new StringBuilder();
bindings.getMemberKeys().forEach(s -> sb.append(s).append("=undefined;"));
eval(sb.toString());

You may wonder why I don't suggest to use delete i.e.

bindings.getMemberKeys().forEach(s -> sb.append("delete ").append(s).append(';'));

That's because I found that we do not specialize well the delete performed on a global object. I have fixed this bug already, but I don't expect you to use the latest development build. So, the delete variant is (unfortunately) slow in the latest release still.

In fact, it may be redundant to do anything in close(). If you decide to write all values for every script then you can keep close() empty. Actually, writing the same set of properties/members may be a good idea because it leads to the same set of "hidden classes/shapes" of the global object. So, the locations (where the value should be written) are kept consistent for all scripts.

Finally, I noticed that your example runs in interpreter mode only (without runtime compilation). I hope that this is just to keep the example simple. If you care about performance then you should run with runtime compilation, of course.

When I applied the mentioned suggestions then the execution time of your example decreased from ~30s to ~0.35s on my machine.

[JonHenson]

Thank you!

Using a script to bind the values has improved performance dramatically. I've modified my code (and committed to the repo) to time 300,000 evaluations (originally 30,000) and to use runtime compilation and execution after warmup is - for me - ~1.5 seconds with GraalJS vs 2 seconds with Rhino.

The reason I was using interpreter only mode is that I'm only able to execute with compilation enabled when running from my IDE. With an uber jar I get an exception saying: No language and polyglot implementation was found on the class-path but that was lower down my list of things to solve than bindings performance - guess it's now risen to the top!

$ mvn clean package &&
> "$JAVA_HOME/bin/java" -Dpolyglotimpl.TraceClassPathIsolation=true -jar target/binding-test.jar
[INFO] Scanning for projects...
[INFO]
[INFO] -----------------------< jon.test:js-perf-test >------------------------
[INFO] Building js-perf-test 0.0.1-SNAPSHOT
[INFO] --------------------------------[ jar ]---------------------------------
:
:
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  6.939 s
[INFO] Finished at: 2024-01-12T10:57:43Z
[INFO] ------------------------------------------------------------------------
[class-path-isolation] Start polyglot class-path isolation
[class-path-isolation] Collected 1 class-path entries from java.class.path system property
[class-path-isolation] Class-path entry: target\binding-test.jar
[class-path-isolation] No truffle-api and/or polyglot found on classpath.
Exception in thread "main" java.lang.IllegalStateException: No language and polyglot implementation was found on the class-path. Make sure at last one language is added on the class-path. If you put a language on the class-path and you encounter this error then there could be a problem with isolated class loading. Use -Dpolyglotimpl.TraceClassPathIsolation=true to debug class loader islation problems. For best performance it is recommended to use polyglot from the module-path instead of the class-path.
        at org.graalvm.polyglot.Engine$PolyglotInvalid.noPolyglotImplementationFound(Engine.java:2071)
        at org.graalvm.polyglot.Engine$PolyglotInvalid.createHostAccess(Engine.java:2057)
        at org.graalvm.polyglot.Engine$PolyglotInvalid.createHostAccess(Engine.java:2023)
        at org.graalvm.polyglot.Engine$Builder.build(Engine.java:753)
        at org.graalvm.polyglot.Engine.create(Engine.java:346)
        at jon.test.js.LazyBinding.graalEvaluatorFactory(LazyBinding.java:85)
        at jon.test.js.LazyBinding.main(LazyBinding.java:296)

[JonHenson]

@iamstolis Thanks for your advice on all of this, it took me longer to circle back to it than I had hoped it would.

I ended up dealing with "unbinding" by evaluating:

Object.keys(globalThis).forEach(k=>globalThis[k]=undefined)

From your comment, it seems that using delete may be the way to go in the future; however, using delete currently seems to have no effect (the variable remains in the bindings map with its value unchanged).

(Incidentally, the issue with running in non-interpreted mode was resolved by moving to the 21.0.2 VM)