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

Propagate match only in asset selection to reduce allocations when grouping #5944

Merged
merged 6 commits into from
Aug 1, 2024

Conversation

nkolev92
Copy link
Member

@nkolev92 nkolev92 commented Jul 31, 2024

Bug

Related: NuGet/Home#12728

Description

The assets selection in restore tends to be 2 phase, group, and then select the frameworks you care about.
The grouping part never really uses the LockFileItem, the strings, the metadata etc. There's already some code that avoids actualizing and create the lock file items for that.
This PR takes that idea and applies to a lot more of places we might allocate unused strings.

This gets us about ~15MB on OrchardCore.
If you look at the traces, the difference is in TryLookup. There's a lot fewer allocations, since strings are not being actualized if they're not going to be needed.

Before:

Name                                                                                                                                                                                                                     	Inc %	          Inc
 nuget.packaging!NuGet.ContentModel.Infrastructure.PatternExpression+TokenSegment.TryMatch(class NuGet.ContentModel.ContentItem&,class System.String,class System.Collections.Generic.IReadOnlyDictionary`2,int32,int32&)	  1.1	   99,485,616
+ mscorlib.ni!System.Collections.Generic.Dictionary`2[System.__Canon,System.__Canon].Insert(System.__Canon, System.__Canon, Boolean)                                                                                     	  0.4	   37,982,336
+ nuget.packaging!ContentPropertyDefinition.TryLookup                                                                                                                                                                    	  0.3	   22,924,920
+ nuget.packaging!NuGet.ContentModel.ContentItem.get_Properties()                                                                                                                                                        	  0.2	   16,821,344
+ system.memory.ni!?                                                                                                                                                                                                     	  0.1	   11,678,544
+ clr!JIT_New                                                                                                                                                                                                            	  0.1	   10,078,472

Name                                                                                                                                                                                                                     	Inc %	          Inc
 nuget.packaging!NuGet.ContentModel.Infrastructure.PatternExpression+TokenSegment.TryMatch(class NuGet.ContentModel.ContentItem&,class System.String,class System.Collections.Generic.IReadOnlyDictionary`2,int32,int32&)	  1.1	   99,325,240
+ mscorlib.ni!System.Collections.Generic.Dictionary`2[System.__Canon,System.__Canon].Insert(System.__Canon, System.__Canon, Boolean)                                                                                     	  0.4	   36,625,432
+ nuget.packaging!ContentPropertyDefinition.TryLookup                                                                                                                                                                    	  0.2	   21,512,424
+ nuget.packaging!NuGet.ContentModel.ContentItem.get_Properties()                                                                                                                                                        	  0.2	   20,301,392
+ clr!JIT_New                                                                                                                                                                                                            	  0.1	   10,511,528
+ system.memory.ni!?                                                                                                                                                                                                     	  0.1	   10,374,464

Name                                                                                                                                                                                                                     	Inc %	          Inc
 nuget.packaging!NuGet.ContentModel.Infrastructure.PatternExpression+TokenSegment.TryMatch(class NuGet.ContentModel.ContentItem&,class System.String,class System.Collections.Generic.IReadOnlyDictionary`2,int32,int32&)	  1.0	   92,317,912
+ nuget.packaging!PatternExpression.Match                                                                                                                                                                                	  1.0	   92,317,912
 + nuget.packaging!ContentItemCollection.PopulateItemGroups                                                                                                                                                              	  0.6	   58,508,488
 + nuget.packaging!ContentItemCollection.FindItemsImplementation                                                                                                                                                         	  0.4	   33,809,424

After:

Name                                                                                                                                                                                                                     	Inc %	          Inc
 nuget.packaging!NuGet.ContentModel.Infrastructure.PatternExpression+TokenSegment.TryMatch(class NuGet.ContentModel.ContentItem&,class System.String,class System.Collections.Generic.IReadOnlyDictionary`2,int32,int32&)	  1.0	   88,885,968
+ mscorlib.ni!System.Collections.Generic.Dictionary`2[System.__Canon,System.__Canon].Insert(System.__Canon, System.__Canon, Boolean)                                                                                     	  0.4	   36,773,904
+ nuget.packaging!NuGet.ContentModel.ContentItem.get_Properties()                                                                                                                                                        	  0.2	   18,401,392
+ nuget.packaging!ContentPropertyDefinition.TryLookup                                                                                                                                                                    	  0.1	   12,331,104
+ system.memory.ni!?                                                                                                                                                                                                     	  0.1	   12,324,000
+ clr!JIT_New                                                                                                                                                                                                            	  0.1	    9,055,568

Name                                                                                                                                                                                                                     	Inc %	          Inc
 nuget.packaging!NuGet.ContentModel.Infrastructure.PatternExpression+TokenSegment.TryMatch(class NuGet.ContentModel.ContentItem&,class System.String,class System.Collections.Generic.IReadOnlyDictionary`2,int32,int32&)	  0.9	   82,424,088
+ mscorlib.ni!System.Collections.Generic.Dictionary`2[System.__Canon,System.__Canon].Insert(System.__Canon, System.__Canon, Boolean)                                                                                     	  0.4	   33,274,936
+ nuget.packaging!NuGet.ContentModel.ContentItem.get_Properties()                                                                                                                                                        	  0.2	   17,833,672
+ nuget.packaging!ContentPropertyDefinition.TryLookup                                                                                                                                                                    	  0.1	   11,798,680
+ clr!JIT_New                                                                                                                                                                                                            	  0.1	    9,822,448
+ system.memory.ni!?                                                                                                                                                                                                     	  0.1	    9,694,352

Name                                                                                                                                                                                                                     	Inc %	          Inc
 nuget.packaging!NuGet.ContentModel.Infrastructure.PatternExpression+TokenSegment.TryMatch(class NuGet.ContentModel.ContentItem&,class System.String,class System.Collections.Generic.IReadOnlyDictionary`2,int32,int32&)	  0.9	   81,275,080
+ mscorlib.ni!System.Collections.Generic.Dictionary`2[System.__Canon,System.__Canon].Insert(System.__Canon, System.__Canon, Boolean)                                                                                     	  0.4	   32,063,952
+ nuget.packaging!NuGet.ContentModel.ContentItem.get_Properties()                                                                                                                                                        	  0.2	   19,575,008
+ system.memory.ni!?                                                                                                                                                                                                     	  0.1	   11,047,056
+ nuget.packaging!ContentPropertyDefinition.TryLookup                                                                                                                                                                    	  0.1	    9,914,472
+ clr!JIT_New                                                                                                                                                                                                            	  0.1	    8,674,592

Name                                                                                                                                                                                                                     	Inc %	          Inc
 nuget.packaging!NuGet.ContentModel.Infrastructure.PatternExpression+TokenSegment.TryMatch(class NuGet.ContentModel.ContentItem&,class System.String,class System.Collections.Generic.IReadOnlyDictionary`2,int32,int32&)	  1.0	   92,817,256
+ mscorlib.ni!System.Collections.Generic.Dictionary`2[System.__Canon,System.__Canon].Insert(System.__Canon, System.__Canon, Boolean)                                                                                     	  0.4	   39,193,360
+ nuget.packaging!NuGet.ContentModel.ContentItem.get_Properties()                                                                                                                                                        	  0.2	   20,815,240
+ system.memory.ni!?                                                                                                                                                                                                     	  0.1	   12,366,792
+ nuget.packaging!ContentPropertyDefinition.TryLookup                                                                                                                                                                    	  0.1	   11,267,464
+ clr!JIT_New                                                                                                                                                                                                            	  0.1	    9,174,400


Microbenchmarks confirm this:

Method Mean Error StdDev Gen0 Gen1 Allocated
NJ 44.10 us 0.164 us 0.153 us 4.3335 0.1221 73.78 KB
MicrosoftBuildRuntime 66.96 us 0.285 us 0.267 us 8.9111 0.7324 151.6 KB
AllPackages 155,316.21 us 563.189 us 526.807 us 15250.0000 1750.0000 259587.62 KB

Match Only:

Method Mean Error StdDev Gen0 Gen1 Allocated
NJ 43.08 us 0.109 us 0.102 us 4.1504 0.1221 70.78 KB
MicrosoftBuildRuntime 65.93 us 0.315 us 0.280 us 8.4229 0.7324 142.8 KB
AllPackages 152,519.29 us 284.897 us 237.902 us 14250.0000 1500.0000 243837.9 KB

Note that from a microbenchmark perspective, the "grouping" step, is:

Method Mean Error StdDev Gen0 Gen1 Allocated
NJ 19.85 us 0.068 us 0.057 us 1.8311 - 31.27 KB
MicrosoftBuildRuntime 21.15 us 0.103 us 0.096 us 1.4648 0.0305 24.68 KB
AllPackages 68,242.99 us 423.168 us 395.832 us 5875.0000 250.0000 99537.28 KB

PR Checklist

  • Meaningful title, helpful description and a linked NuGet/Home issue
  • Added tests
  • Link to an issue or pull request to update docs if this PR changes settings, environment variables, new feature, etc.

@nkolev92 nkolev92 changed the title Propagate match only to minimize allocations when grouping Propagate match only in asset selection to minimize allocations when grouping Jul 31, 2024
@nkolev92 nkolev92 changed the title Propagate match only in asset selection to minimize allocations when grouping Propagate match only in asset selection to reduce allocations when grouping Jul 31, 2024
@nkolev92 nkolev92 marked this pull request as ready for review July 31, 2024 22:59
@nkolev92 nkolev92 requested a review from a team as a code owner July 31, 2024 22:59
@nkolev92 nkolev92 merged commit 3b318e3 into dev Aug 1, 2024
28 checks passed
@nkolev92 nkolev92 deleted the dev-nkolev92-propagateMatchOnly branch August 1, 2024 19:24
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