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

Fix quadratic algorithm in CompilerGeneratedState #3150

Merged
merged 3 commits into from
Dec 10, 2022

Conversation

agocke
Copy link
Member

@agocke agocke commented Dec 6, 2022

The way this code was supposed to work was that it would scan the compiler- generated type and all its descendents, record each generated type it found, then fill in information for all of the found types. The way it actually worked was that it would scan the descendents, record each generated type, then try to fill in information for all generated types found in the program. This is quadratic as you start adding types, as you rescan everything you've added before. The fix is to record just the types from the current pass, and then add them to the larger bag when everything's complete.

The way this code was supposed to work was that it would scan the compiler-
generated type and all its descendents, record each generated type it found,
then fill in information for all of the found types. The way it actually
worked was that it would scan the descendents, record each generated type, then
try to fill in information *for all generated types found in the program*. This
is quadratic as you start adding types, as you rescan everything you've added
before. The fix is to record just the types from the current pass, and then
add them to the larger bag when everything's complete.
@agocke agocke requested a review from marek-safar as a code owner December 6, 2022 08:31
@agocke agocke requested a review from sbomer December 6, 2022 08:31
@agocke
Copy link
Member Author

agocke commented Dec 6, 2022

This isn't quite ready -- I have to create some test that will demonstrate the problem and verify the fix.

@vitek-karas
Copy link
Member

Would be interesting to get some perf numbers to see how much this helps.

@agocke
Copy link
Member Author

agocke commented Dec 7, 2022

Turns out this change is less impactful than I thought. I created a test program with 10000 classes, each with one async method and one lambda -- generating ~20000 types. It looks like the ILLink time goes from 15s to 9.5s.

Overall, worth doing, but this will likely only make a significant difference for programs with lots and lots of generated types.

There's no way this could be a useful test case when running constantly -- I'll probably try to find some place to check it in and leave it disabled, so in case anyone wants to make any improvements here they can run the test and give it a try.

Copy link
Member

@sbomer sbomer left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@agocke agocke merged commit 27ce032 into dotnet:main Dec 10, 2022
@agocke agocke deleted the fix-quadratic-type-walk branch December 10, 2022 05:16
tlakollo pushed a commit to tlakollo/linker that referenced this pull request Dec 20, 2022
The way this code was supposed to work was that it would scan the compiler-
generated type and all its descendants, record each generated type it found,
then fill in information for all of the found types. The way it actually
worked was that it would scan the descendants, record each generated type, then
try to fill in information *for all generated types found in the program*. This
is quadratic as you start adding types, as you rescan everything you've added
before. The fix is to record just the types from the current pass, and then
add them to the larger bag when everything's complete.

Commit migrated from dotnet@27ce032
tlakollo pushed a commit to tlakollo/runtime that referenced this pull request Dec 22, 2022
The way this code was supposed to work was that it would scan the compiler-
generated type and all its descendants, record each generated type it found,
then fill in information for all of the found types. The way it actually
worked was that it would scan the descendants, record each generated type, then
try to fill in information *for all generated types found in the program*. This
is quadratic as you start adding types, as you rescan everything you've added
before. The fix is to record just the types from the current pass, and then
add them to the larger bag when everything's complete.

Commit migrated from dotnet/linker@27ce032
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.

3 participants