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.

Sign up for GitHub

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

Jump to bottom

Reenable consumption of PGO data #91297

Closed
wants to merge 1 commit into from
Closed

Reenable consumption of PGO data #91297

wants to merge 1 commit into from

Conversation

AustinWise
Copy link
Contributor

This reverts commit 810231c from #89311. Once the PGO data has been updated with the new linker flags and CI passes on this PR, this can be merged to reenable PGO.

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Aug 29, 2023
@ghost
Copy link

ghost commented Aug 29, 2023

Tagging subscribers to this area: @hoyosjs
See info in area-owners.md if you want to be subscribed.

Issue Details

This reverts commit 810231c from #89311. Once the PGO data has been updated with the new linker flags and CI passes on this PR, this can be merged to reenable PGO.

Author: AustinWise
Assignees: -
Labels:

area-Infrastructure-coreclr
Milestone: -

@AustinWise AustinWise force-pushed the austin/DllLoading branch 2 times, most recently from db31380 to 3aa7311 Compare September 3, 2023 15:25
@AustinWise AustinWise marked this pull request as draft September 3, 2023 15:26
@elinor-fung elinor-fung mentioned this pull request Sep 7, 2023
Closed
@DrewScoggins
Copy link
Member

Sorry, this didn't get updated. We have had updated PGO data for the last three weeks.

@jeffschwMSFT
Copy link
Member

now that runtime official build has over come an infra issue, this issue is now the top cause of build breaks. what is the ETA for getting it merged?

@AustinWise
Copy link
Contributor Author

AustinWise commented Sep 7, 2023 •
edited
Loading

I think #91497 might need to be merged first. I'll cherry pick the changes from that PR into this one to see if that is the case.

@elinor-fung
Copy link
Member

@jeffschwMSFT what build breaks is this causing? #89311 went in last week (29-AUG) and we've had successful builds since then.

@elinor-fung
Copy link
Member

If you are talking about the LNK1264 error, I expect that is from: #91563 - specifically #91563 (comment)

@AustinWise
Copy link
Contributor Author

It looks like #91497 does not currently contain the updated linker flags that would allow this PR to be merged.

This was referenced Sep 8, 2023
Closed
Closed
@AustinWise AustinWise force-pushed the austin/DllLoading branch 2 times, most recently from 8fe1378 to 920646b Compare September 14, 2023 20:15
@build-analysis build-analysis bot mentioned this pull request Sep 14, 2023
Closed
@DrewScoggins
Copy link
Member

Is this ready to merge?

@AustinWise
Copy link
Contributor Author

@DrewScoggins It can be merged once the main branch contains PGO data that was produced from the main branch. Currently it appears the PGO data is being built from the 8.0 branch, which is not compatible. See #93280 for an example of PR that updates the PGO data.

@build-analysis build-analysis bot mentioned this pull request Oct 28, 2023
Closed
@build-analysis build-analysis bot mentioned this pull request Nov 15, 2023
Closed
This was referenced Nov 27, 2023
Closed
Closed
@amanasifkhalid amanasifkhalid mentioned this pull request Dec 1, 2023
Closed
@amanasifkhalid
Copy link
Member

Hi all, #95510 was recently merged. Is this PR ready to be merged?

@jkotas jkotas closed this Dec 1, 2023
@jkotas jkotas reopened this Dec 1, 2023
@jkotas
Copy link
Member

jkotas commented Dec 1, 2023

Hi all, #95510 was recently merged. Is this PR ready to be merged?

I can be merged once the CI is green.

@AustinWise
Copy link
Contributor Author

It is still failing with this error:

LINK : fatal error LNK1268: inconsistent option 'DEPENDENTLOADFLAG:0x800' specified with /USEPROFILE but not with /GENPROFILE

I'm guessing the PGO data is being built from the dotnet/runtime 8.0 branch, which does not have the DEPENDENTLOADFLAG. Someone at Microsoft would need to update the dnceng/internal/dotnet-optimization repo to build from the dotnet/runtime main branch to fix the problem.

@AustinWise
Copy link
Contributor Author

Closing in favor of #95540 .

@AustinWise AustinWise closed this Dec 2, 2023
@AustinWise AustinWise deleted the austin/DllLoading branch December 2, 2023 23:15
@github-actions github-actions bot locked and limited conversation to collaborators Jan 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
area-Infrastructure-coreclr community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@AustinWise @DrewScoggins @jeffschwMSFT @elinor-fung @amanasifkhalid @jkotas