Skip to content

Conversation

@mangod9
Copy link
Member

@mangod9 mangod9 commented Jun 20, 2021

Should improve build times further.

Should improve build times further.
@ghost
Copy link

ghost commented Jun 20, 2021

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

Issue Details

Should improve build times further.

Author: mangod9
Assignees: -
Labels:

area-Infrastructure-coreclr

Milestone: -

@mangod9
Copy link
Member Author

mangod9 commented Jun 21, 2021

cc @dotnet/crossgen-contrib . This should remove about ~80 files from compiling.

@mangod9
Copy link
Member Author

mangod9 commented Jun 21, 2021

win-arm64 failures are due to overall helix arm64 flakiness.

Copy link
Member

@davidwrighton davidwrighton left a comment

Choose a reason for hiding this comment

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

LGTM, but you can probably remove more. In a future PR, I would see if you can remove all the files in src/coreclr/jit/crossgen, as well as the cmakelists.txt in src/coreclr/zap

Copy link
Member

@hoyosjs hoyosjs left a comment

Choose a reason for hiding this comment

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

We should give a heads up to Samsung in crossgen cleanup. I believe they were still using this. Linking #54129

cc: @alpencolt @gbalykov @clamp03

@mangod9
Copy link
Member Author

mangod9 commented Jun 22, 2021

Yeah will link to #54129 once merged.

@mangod9
Copy link
Member Author

mangod9 commented Jun 22, 2021

LGTM, but you can probably remove more. In a future PR, I would see if you can remove all the files in src/coreclr/jit/crossgen, as well as the cmakelists.txt in src/coreclr/zap

yeah will remove more cmakelists in subsequent PRs, but dont believe they are being exercised so wouldnt affect build times.

@mangod9 mangod9 merged commit a2f5817 into dotnet:main Jun 22, 2021
@clamp03
Copy link
Member

clamp03 commented Jun 23, 2021

@hoyosjs @mangod9 Thank you for your consideration.

@alpencolt
Copy link

We should give a heads up to Samsung in crossgen cleanup. I believe they were still using this. Linking #54129

cc: @alpencolt @gbalykov @clamp03

We haven't decided what will use in .net6, but according to to #54129 crossge1 will be removed in .net7, am I right? If so it's ok.

@mangod9
Copy link
Member Author

mangod9 commented Jun 25, 2021

Hey @alpencolt, crossgen has been removed from building in the runtime repo, and we might start removing (the now defunct) code post .net preview7, but could defer till start of .net 7 if that helps.

@ghost ghost locked as resolved and limited conversation to collaborators Jul 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants