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

Re-architecture quote pickling #12540

Merged
merged 2 commits into from
Apr 20, 2022

Conversation

nicolasstucki
Copy link
Contributor

@nicolasstucki nicolasstucki commented May 20, 2021

Separate the logic that creates holes in quotes from the logic that pickles the quotes. Holes are created in the Splicer phase and the result of the transformation can be -Ychecked. Now, the PickleQuotes phase only needs to extract the contents of the holes, pickle the quote and put them into a call to unpickleExprV2/unpickleTypeV2. See documentation in PickleQuotes and Splicer.

We add unpickleExprV2/unpickleTypeV2 to support some optimization in the encoding of the pickled quote. Namely, we removed an unnecessary lambda from the arguments of the hole passed into the contents of the hole. By not changing unpickleExpr/unpickleType the current compiler will be able to handle the old encoding in binaries compiled with older compilers.

Fixes #8100
Fixes #12440
Fixes #13563
Fixes #13732
Fixes #14337
Fixes #14373

@nicolasstucki nicolasstucki self-assigned this May 20, 2021
@nicolasstucki nicolasstucki force-pushed the rearchitecture-quote-splitting branch 8 times, most recently from d35cb27 to a3f0700 Compare May 21, 2021 12:02
@nicolasstucki nicolasstucki force-pushed the rearchitecture-quote-splitting branch 3 times, most recently from 345af46 to 1039b5b Compare June 10, 2021 08:01
@nicolasstucki nicolasstucki force-pushed the rearchitecture-quote-splitting branch 3 times, most recently from 216e1b8 to 8d634fe Compare June 10, 2021 09:58
@nicolasstucki nicolasstucki force-pushed the rearchitecture-quote-splitting branch 4 times, most recently from 3e979f5 to fc1bf94 Compare July 29, 2021 11:44
@nicolasstucki nicolasstucki force-pushed the rearchitecture-quote-splitting branch 2 times, most recently from 39e2c88 to 11cd4c4 Compare August 30, 2021 15:29
@nicolasstucki nicolasstucki force-pushed the rearchitecture-quote-splitting branch 2 times, most recently from 11cd4c4 to 70ddaa6 Compare October 8, 2021 09:11
@nicolasstucki nicolasstucki linked an issue Oct 12, 2021 that may be closed by this pull request
@dottybot
Copy link
Member

dottybot commented Feb 4, 2022

Performance test finished successfully:

Visit https://dotty-bench.epfl.ch/12540/ to see the changes.

Benchmarks is based on merging with main (4c7ec9b)

@nicolasstucki
Copy link
Contributor Author

test performance with #quotes please

@dottybot
Copy link
Member

dottybot commented Feb 4, 2022

performance test scheduled: 1 job(s) in queue, 0 running.

@dottybot
Copy link
Member

dottybot commented Feb 4, 2022

Performance test finished successfully:

Visit https://dotty-bench.epfl.ch/12540/ to see the changes.

Benchmarks is based on merging with main (4c7ec9b)

@nicolasstucki nicolasstucki force-pushed the rearchitecture-quote-splitting branch 2 times, most recently from 3c2ae12 to b9a5448 Compare March 4, 2022 08:12
@xuwei-k
Copy link
Contributor

xuwei-k commented Mar 18, 2022

@nicolasstucki nicolasstucki added the release-notes Should be mentioned in the release notes label Apr 13, 2022
@nicolasstucki nicolasstucki force-pushed the rearchitecture-quote-splitting branch 2 times, most recently from 336a00f to 0683579 Compare April 19, 2022 15:14
Separate the logic that creates holes in quotes from the logic that
pickles the quotes. Holes are created in the `Splicer` phase and the
result of the transformation can be `-Ycheck`ed. Now, the `PickleQuotes`
phase only needs to extract the contents of the holes, pickle the quote
and put them into a call to `unpickleExprV2`/`unpickleTypeV2`.

We add `unpickleExprV2` to support some optimization in the encoding of
the pickled quote. Namely we removed an unnecessary lambda from the
arguments of the hole passed into the contents of the hole. By not
changing `unpickleExpr` the current compiler will be able to handle the
old encoding in binaries compiled with older compilers.
The `unpickleTypeV2` is just a version of `unpickleType` that does not
take the `termHole` parameter which is always `null`.
With `-Yscala-relese` 3.0 or 3.1, the compiler will generate calls to
the old `unpickleExpr`/`unpickleType`.

Fixes scala#8100
Fixes scala#12440
Fixes scala#13563
Fixes scala#14337
Fixes scala#14373
Closes scala#13732
@nicolasstucki
Copy link
Contributor Author

Finally, we are able to merge this PR. It needs a last review to check that nothing unwanted happened when rebasing.

Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

❇️

@smarter smarter merged commit d5ae77e into scala:main Apr 20, 2022
@smarter smarter deleted the rearchitecture-quote-splitting branch April 20, 2022 09:52
@Kordyjan Kordyjan modified the milestones: 3.2.0-RC1, 3.2.0 Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-minor-release This PR cannot be merged until the next minor release release-notes Should be mentioned in the release notes
Projects
None yet
6 participants