Karate 1.4 takes significantly longer to run tests than 1.2

[ericdriggs]

@ptrthomas
Karate 1.4 is still significantly slower than 1.2 for large projects.
We're seeing between a +20% to +100% increase in time to run tests.
Haven't yet been able to identify most common examples of why tests are running longer.
If I run a benchmark on 1.2 vs 1.4 I'll submit results here

Likely related to synchronization in:
#2204

Not sure if possible to do a deep copy of context to minimize synchronization code.

[ptrthomas]

related issue to investigate: https://stackoverflow.com/a/76285986/143475

recommendation is to use Java instead of JS functions where possible. one suggestion to help you troubleshoot is place some obvious logging around the lock and then you can scan the logs to see how many times - and where in your tests it is invoked. for example, if you do ScenarioEngine.get() you can get the "active" scenario and introspect the feature, karate call stack etc.

will investigate, but this may take a while. I would recommend refactoring to Java, it may needed only for a few hotspots, most likely utility functions you may be using @ericdriggs

[ptrthomas]

the lock is over here: https://github.com/karatelabs/karate/blob/v1.4.0/karate-core/src/main/java/com/intuit/karate/graal/JsValue.java#L307

[ericdriggs]

To summarize concurrency issue

PolglotContextImpl::throwDeniedAccess is called when multi-threaded access occurs for languages which don't have isThreadAccessAllowed == true

 * By default every {@link #createContext(Env) context} only allows access from one thread at the
 * same time. Therefore if the context is tried to be accessed from multiple threads at the same
 * time the access will fail. Languages that want to allow multi-threaded access to a context may
 * override {@link #isThreadAccessAllowed(Thread, boolean)} and return <code>true</code> also for
 * multi-threaded accesses. Initialization actions for multi-threaded access can be performed by
 * overriding {@link #initializeMultiThreading(Object)}. Threads are
 * {@link #initializeThread(Object, Thread) initialized} and {@link #disposeContext(Object)
 * disposed} before and after use with a context. Languages may
 * {@link Env#newTruffleThreadBuilder(Runnable) create} new threads if the environment
 * {@link Env#isCreateThreadAllowed() allows} it.

public abstract class TruffleLanguage

https://github.com/oracle/graal/blob/master/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/TruffleLanguage.java

Language implementation in graal-js doesn't override this value, so concurrent access throws.

It's possible to either fork/hard-code override this implementation, or to register a new version of js language which allows multi-threaded access, but doing so isn't supported and may introduce additional concurrency issues because graal is intentionally not thread safe because

1. avoids locking overhead and
2. their interpretation of ecma262 is that js is designed to be single threaded due to eval semantics

can i change com.oracle.truffle.api.LanguageAccessor.LanguageImpl#isThreadAccessAllowed(com.oracle.truffle.api.TruffleLanguage.Env, java.lang.Thread, boolean) method for return always true? is there any example code, that fails after changing isThreadAccessAllowed method?

If you do, all support from our side is off. We don't give any guarantees what works if you change that. While you should be able to secure your own classes by proper synchronization, problems might occur in core classes like JavaScript's Object model (as stated above already).

So yes, that might work for some cases, but you will eventually run into typical multithreading problems like lost updates, corrupted objects, potentially even deadlocks. Again: this is a model we neither endorse nor support. You have been warned. The models we support have been quoted above.

...

The right body to address this criticism to is the ECMAScript TC39, in charge of the ECMAScript/JavaScript specification. JavaScript, by design, is specified with single-threaded evaluation semantics. Unlike other languages, it is NOT designed to allow full multi-threading. There are some models still possible (as linked above).

oracle/graaljs#481

see also:

An execution context is a specification device that is used to track the runtime evaluation of code by an ECMAScript implementation. At any point in time, there is at most one execution context per agent that is actually executing code.

https://tc39.es/ecma262/#sec-execution-contexts

Relevant Workarounds:
A. one share-nothing context per thread
B. synchronized access to js objects using single context for multiple threads
C. use java concurrency
D. use java objects
https://github.com/oracle/graaljs/blob/master/docs/user/Multithreading.md#multithreading-with-java-and-javascript

It seems that karate is currently using B. It seems that if A is possible through use of deep copies or java objects, this would eliminate almost all concurrent access.

[ptrthomas]

@ericdriggs cc @joelpramos I hope I've figured out a reasonable solution. here we are discussing JS functions:

• we avoid the lock by default, which is great
• when we detect that the JS function originated in another scenario, we re-hydrate it, and avoid the lock again
• since re-hydrating will not work when you do function "gymnastics" or refer to other functions in your function (see pic below) we introduce a karate.wrapFunction() helper. this will lock across all threads, so using this is at your own risk and you should try to use Java instead

would be great if you build locally and test

[ptrthomas]

@eugeniooliveira would you be able to build locally and check if this solves the problem you reported here: https://stackoverflow.com/q/76284686/143475

[ptrthomas]

another related conversation and fix attempt in the past: #2222

[ptrthomas]

all: note that 1.4.1.RC3 is available in maven central for those who would like to test this

[ericdriggs]

@ptrthomas
It seems that what you are saying is that functions will need to use karate.wrapFunction to see performance improvements.
This doesn't seem like it's directly addressing the concurrency performance penalty if it requires code refactoring.

Not sure why karate 1.2 seems to have strictly better performance in JS than the latest version, but that's an extremely compelling case to not upgrade.

[ptrthomas]

@ericdriggs no, actually most functions will NOT need karate.wrapFunction(). to clarify, you need to use karate.wrapFunction() only in cases where you depend on a JS function (variable) within a JS function body. who knows, you may need zero refactoring

[ptrthomas]

1.4.1 released

[ptrthomas]

@ericdriggs @joelpramos FYI about our intent to create a lightweight JS engine from scratch to replace Graal: https://twitter.com/ptrthomas/status/1775754727700996381

can't wait to get rid of 60MB of the JARs and get the exact behavior we want

[joelpramos]

Good stuff - makes lot of sense. Hopefully you can keep some sort of backwards compatibility / common interfaces as if one day Graal - or another one - is back on the table as an option. I always thought that the potential was there to allow exploring bringing NodeJS packages onto Karate and/or porting Karate into node etc.

[ericdriggs]

Bravo, bravo, bravissimo!