-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Parallelize position pickling #9619
Parallelize position pickling #9619
Conversation
Run position pickling and byte array assembly in a separate task
# Conflicts: # compiler/src/dotty/tools/dotc/transform/Pickler.scala
test performance please |
performance test scheduled: 16 job(s) in queue, 1 running. |
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/9619/ to see the changes. Benchmarks is based on merging with master (387c562) |
pickled | ||
}(using ExecutionContext.global) | ||
def force(): Array[Byte] = Await.result(pickledF, Duration.Inf) | ||
if ctx.settings.YtestPickler.value || ctx.mode.is(Mode.Interactive) then force() |
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 tried removing the special-case for Interactive mode here and the CI passed: https://github.com/smarter/dotty/actions/runs/219013322, are you still able to reproduce the issue you saw that lead to that change?
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.
If the CI passes I assume it's OK, and we can do without the restriction.
@oderky it looks like this PR broke master. There is some issue when running the REPL tests. |
This disables #9619 as it lead to CI failures.
This disables #9619 as it lead to CI failures.
Run position pickling in parallel with other middle-end transformations. Only force in backend.