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

Enable PGO #17286

Closed
wants to merge 7 commits into from
Closed

Enable PGO #17286

wants to merge 7 commits into from

Conversation

psfinaki
Copy link
Member

@psfinaki psfinaki commented Jun 6, 2024

Description

Fixes #12636
Fixes #13328

Benchmarks

I published FSI and FSC locally for net 8.0 and win-64 runtime, then ran them against an empty F# script.

FSI results

image

Before:

First run: ~2.9 s
Later runs: ~1.4 s

After:

First run: ~1.7 s
Later runs: ~0.9 s

FSC results

image

Before:

First run: ~2.9 s
Later runs: ~1.9 s

After:

First run: ~1.8 s
Later runs: ~1.0 s


The improvements are about 40% in all cases!

Checklist

  • Test cases added
  • Performance benchmarks added in case of performance changes

Copy link
Contributor

github-actions bot commented Jun 6, 2024

✅ No release notes required

@psfinaki psfinaki requested a review from EgorBo June 6, 2024 16:34
src/fsc/fscProject/fsc.fsproj Outdated Show resolved Hide resolved
src/fsi/fsiProject/fsi.fsproj Outdated Show resolved Hide resolved
@psfinaki
Copy link
Member Author

/azp run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@psfinaki psfinaki marked this pull request as ready for review June 12, 2024 16:56
@psfinaki psfinaki requested a review from a team as a code owner June 12, 2024 16:56
@psfinaki psfinaki changed the title [WIP] Enable PGO Enable PGO Jun 12, 2024
@psfinaki psfinaki marked this pull request as draft June 12, 2024 17:12
@psfinaki psfinaki marked this pull request as ready for review June 13, 2024 15:21
Copy link
Member

@vzarytovskii vzarytovskii left a comment

Choose a reason for hiding this comment

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

We shouldn't be producing our own profiles, but consume ones which are produced for us, and which we consume via darc

@vzarytovskii
Copy link
Member

@EgorBo we don't really want to do the profile generation ourselves, can't we just consume what you're publishing already for fsi and contribute some tests for the fsc?

@EgorBo
Copy link
Member

EgorBo commented Jun 13, 2024

@EgorBo we don't really want to do the profile generation ourselves, can't we just consume what you're publishing already for fsi and contribute some tests for the fsc?

I guess it's up to you, I'd also recommend consuming the MIBC from nuget and use the same workflow that we use in dotnet/runtime where training is done in a separate repo. I thought that you decided to do the training here in this repo, if you don't then I'd also vote for just consuming the MIBC from the nuget

@dotnet dotnet deleted a comment from github-actions bot Jun 13, 2024
@vzarytovskii
Copy link
Member

@EgorBo we don't really want to do the profile generation ourselves, can't we just consume what you're publishing already for fsi and contribute some tests for the fsc?

I guess it's up to you, I'd also recommend consuming the MIBC from nuget and use the same workflow that we use in dotnet/runtime where training is done in a separate repo. I thought that you decided to do the training here in this repo, if you don't then I'd also vote for just consuming the MIBC from the nuget

Yeah, I'd say it would be easier for us to consume ready to use profiles, in case if something changes in generating them, we don't have to adjust ours

@dotnet dotnet deleted a comment from github-actions bot Jun 13, 2024
@psfinaki psfinaki marked this pull request as draft June 13, 2024 17:47
@psfinaki
Copy link
Member Author

Closing in favor of #17317

@psfinaki psfinaki closed this Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

dotnet fsi startup overhead surprisingly high Add mibc/profile for R2R FSC/FSI for faster startup
3 participants