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

write pipelined tasty in parallel. #20153

Merged
merged 3 commits into from
Apr 16, 2024

Conversation

bishabosha
Copy link
Member

@bishabosha bishabosha commented Apr 10, 2024

sets up the executor in pickler to write the files as the final task, meaning we no longer read from unit.pickled, but directly send the tasty bytes to the serialized holder, which queues up the internal name string of the class, and its bytes. When writing TASTy asynchronously, collect any logs into a buffer.

When the task for writing is finished, complete a promise with info describing if the writing was successful, and any pending log messages.

when the promise completes, and no errors occur in writing tasty, then asynchronously call the Zinc callbacks which trigger downstream pipelined compilation.

await on the promise in GenBCode, at which point report any buffered errors

fixes #20101

@bishabosha bishabosha requested a review from sjrd April 10, 2024 16:09
@bishabosha
Copy link
Member Author

bishabosha commented Apr 10, 2024

I have yet to benchmark any theoretical performance gain - but I would assume it to be fairly trivial to predict on main branch by measuring how long the present writeSigFiles in ExtractAPI blocks for.

compiler/src/dotty/tools/backend/jvm/GenBCode.scala Outdated Show resolved Hide resolved
compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala Outdated Show resolved Hide resolved
compiler/src/dotty/tools/dotc/transform/Pickler.scala Outdated Show resolved Hide resolved
compiler/src/dotty/tools/dotc/transform/Pickler.scala Outdated Show resolved Hide resolved
compiler/src/dotty/tools/dotc/transform/Pickler.scala Outdated Show resolved Hide resolved
compiler/src/dotty/tools/io/FileWriters.scala Outdated Show resolved Hide resolved
compiler/src/dotty/tools/io/FileWriters.scala Outdated Show resolved Hide resolved
@bishabosha bishabosha marked this pull request as draft April 11, 2024 16:03
@bishabosha bishabosha force-pushed the topic/zinc-pipelining-async branch from acbb1c6 to 3279a4c Compare April 12, 2024 16:31
@bishabosha
Copy link
Member Author

There should probably be a cancellation signal also

@bishabosha bishabosha marked this pull request as ready for review April 12, 2024 18:17
@bishabosha bishabosha requested a review from sjrd April 12, 2024 18:17
@bishabosha bishabosha force-pushed the topic/zinc-pipelining-async branch from 3279a4c to 97422d9 Compare April 15, 2024 16:55
@bishabosha bishabosha force-pushed the topic/zinc-pipelining-async branch from 97422d9 to 8f5e659 Compare April 15, 2024 17:26
Now in backend we block on the composition of the Zinc API callbacks
and tasty being written.

We also now conditionally signal that we are "done" writing tasty,
based on if any units were suspended. This works in line with the
Zinc default, which will ignore the early output anyway under the
presence of macros (user can override this). Also move async tasty
holder to the Run, as it is now context dependent on suspending.

TODO: give the user an option to optimise performance by preventing definition
of such problematic macros (which would also avoid suspensions).
@bishabosha bishabosha force-pushed the topic/zinc-pipelining-async branch from 8f5e659 to a25d252 Compare April 15, 2024 17:56
@bishabosha
Copy link
Member Author

@sjrd all comments are addressed

Copy link
Member

@sjrd sjrd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK I only have one comment left on the cancellation behavior.

compiler/src/dotty/tools/dotc/transform/Pickler.scala Outdated Show resolved Hide resolved
@bishabosha bishabosha requested a review from sjrd April 16, 2024 16:10
@bishabosha bishabosha merged commit 5f3e1d7 into scala:main Apr 16, 2024
19 checks passed
@bishabosha bishabosha deleted the topic/zinc-pipelining-async branch April 16, 2024 18:06
@Kordyjan Kordyjan added this to the 3.5.0 milestone May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

write pipeline TASTy files asynchronously
4 participants