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

IlxGen parallelization #14372

Merged
merged 53 commits into from
Jan 10, 2023
Merged

IlxGen parallelization #14372

merged 53 commits into from
Jan 10, 2023

Conversation

T-Gro
Copy link
Member

@T-Gro T-Gro commented Nov 22, 2022

This is right now implemented as an opt-in feature that allows parallel run of generating the code of method bodies, later in the IlxGen process.
It is not a full parallelization, since anything besides method body generation still happens eagerly and sequentially.

This can be enabled either by:
--test:ParallelIlxGen command line switch to fsc.exe
Environment variable Environment.GetEnvironmentVariable("FSHARP_EXPERIMENTAL_FEATURES") |> isNotNull

The environment variable option is preferred to get dogfooding as much as we can.

Current timing for ComponentTests project:

Old, sequential approach:
image

Parallel approach (most of the code is still sequential though. That is why the impact is not that big):
image

When implementing this, AssemblyCodeGen had to be made thread safe. While at it, locking has been reduced for certain common global functions.

@T-Gro T-Gro changed the title [WIP] Revive ilxgen parallelization IlxGen parallelization Nov 28, 2022
@T-Gro T-Gro marked this pull request as ready for review November 28, 2022 14:08
@T-Gro T-Gro requested a review from vzarytovskii December 12, 2022 11:31
0101
0101 previously approved these changes Dec 16, 2022
@T-Gro T-Gro requested review from dsyme, KevinRansom and a team December 27, 2022 17:30
psfinaki
psfinaki previously approved these changes Jan 4, 2023
@T-Gro T-Gro dismissed stale reviews from psfinaki and 0101 via b0a94c2 January 5, 2023 11:51
@T-Gro T-Gro requested review from psfinaki and 0101 January 6, 2023 14:47
@T-Gro T-Gro added this to the January-2023 milestone Jan 9, 2023
@T-Gro T-Gro self-assigned this Jan 9, 2023
Copy link
Member

@KevinRansom KevinRansom left a comment

Choose a reason for hiding this comment

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

This looks good. Perhaps have a think about the oversized collections. And also whether there is a good heuristic for sizing collections.
Thanks

Kevin

src/Compiler/AbstractIL/il.fs Show resolved Hide resolved
src/Compiler/TypedTree/CompilerGlobalState.fs Show resolved Hide resolved
@T-Gro
Copy link
Member Author

T-Gro commented Jan 10, 2023

This looks good. Perhaps have a think about the oversized collections. And also whether there is a good heuristic for sizing collections. Thanks

Kevin

Will address this right after this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants