-
Notifications
You must be signed in to change notification settings - Fork 327
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
Reschedule guest code execution into dedicated thread pool #12613
Conversation
…tations can happen later
e3b2be8
to
b591593
Compare
There are three remaining failures in
The rest of the JVM tests seems to be green. |
- based on experiments made in #12613 that are by themselves motivated by #12500 - `TruffleLogger` shall **only be used** in the interpreter running _guest code_ - most of the code in `runtime-instrument-xyz` projects is **not** running a _guest code_ - most of the code in these instruments schedules runs of _guest code_ - thus using `TruffleLogger` isn't appropriate and it yields exception when it cannot find `EnsoContext` around - let's make the code ready for _context_ **not being around** - let's use SLF4J instead of `TruffleLogger`
It flies!
|
@@ -794,33 +792,36 @@ public boolean isGlobalSuggestionsEnabled() { | |||
|
|||
/** The job parallelism or 1 */ | |||
public int getJobParallelism() { | |||
var n = getOption(RuntimeOptions.JOB_PARALLELISM_KEY); | |||
var base = n == null ? 1 : n.intValue(); | |||
var optimal = Math.round(base * 0.5); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I don't really understand this
base * 0.5
logic!? - seems to be result of Improvements that significantly reduce the chances of request timeouts #7042
- it is a PR with long description
- but it doesn't explain why the
base
is being divided into half?
|
override def contextModifiers: Option[Context#Builder => Context#Builder] = | ||
Some(b => { | ||
b.allowCreateThread(true) | ||
.option(RuntimeOptions.JOB_PARALLELISM, "" + parallelism) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- this test no longer allocates its own
new Thread
, but it relies on builtinJOB_PARALLELISM
- this could help a lot with - Improve visualizations feedback via cache observables and safepoints #10525
- enough to increase value of
JOB_PARALLELISM
(default remains1
) - schedule visualization computations as
getThreadManager.submit
- and they will be executed in parallel to regular computation
} | ||
return null; | ||
}); | ||
resultOf(pending); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ExecutionService
keeps compatibility with previous blocking behavior- even the Enso code computation is performed in guest thread
- it has to to address Upgrading to Truffle libraries 24.2.0 #12500 (comment)
- e.g. the code is
submit
ed toThreadManager
for execution Future
is returned- but all methods in
ExecutionService
wait for suchFuture
s to be finished - these methods could be changed to return
Future
in subsequent PRs - that would help with Improve visualizations feedback via cache observables and safepoints #10525
- e.g. this PR opens the door for parallel evaluation of visualizations
Pull Request Description
Enso is using Truffle threads and especially those that allow execution of guest code too much. A lot of other code is running in such threads. However those threads are subject to special control and support from Truffle framework (for example execution of safepoints).
Let's reduce the usage of those Truffle threads to a minimum. Leave the other threads "normal".
Important Notes
ThreadManager.interruptThreads()
is essential for this PRRuntimeAsyncCommandsTest
will not passChecklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,