Skip to content

Re-enable parallel pickling #9650

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

Merged
merged 3 commits into from
Sep 1, 2020
Merged

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Aug 27, 2020

Both position and comment pickler accessed symbols in trees. Position pickler also accessed denotations through symbols.
This is a potential race since both operations depend on phase-indexed caches, which are not protected.

We now avoid the problem by pre-computing everything position and comment pickler need from symbols, and passing it separately to them. Neither position nor tree pickler takes a context anymore, which means they cannot possibly interfere
with the rest -- or, rather, they can interfere only through global variables, the same way a parallel compiler testing thread does. But we have that well under control through the existing static race detection mechanism.

@odersky odersky force-pushed the parallel-pickling branch from c01c96e to 4ecb501 Compare August 27, 2020 09:54
@odersky
Copy link
Contributor Author

odersky commented Aug 27, 2020

OK, so I did re-run the jobs 4 times. Nothing acted up. I propose we merge but only after the next release is cut. /cc @anatoliykmetyuk

println(new TastyPrinter(pickled).printContents())
}
pickled
}(using ExecutionContext.global)
Copy link
Member

Choose a reason for hiding this comment

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

This isn't urgent but instead of hardcoding the ExecutionContext we should probably get it from the Context to allow it to be customized by the build tool if it uses a different thread pool, etc.

@odersky odersky merged commit 834c989 into scala:master Sep 1, 2020
@odersky odersky deleted the parallel-pickling branch September 1, 2020 10:02
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.

2 participants