Skip to content

Perf: Remove LinkedList usage in ItemDictionary#12345

Merged
YuliiaKovalova merged 4 commits intodotnet:mainfrom
ccastanedaucf:dev/chcasta/perf-itemdictionary-nolinkedlist
Oct 27, 2025
Merged

Perf: Remove LinkedList usage in ItemDictionary#12345
YuliiaKovalova merged 4 commits intodotnet:mainfrom
ccastanedaucf:dev/chcasta/perf-itemdictionary-nolinkedlist

Conversation

@ccastanedaucf
Copy link
Contributor

@ccastanedaucf ccastanedaucf commented Aug 14, 2025

NOTE:

Context

After #12320 , the O(1) LinkedListNode lookup in ItemDictionary is only used for removes.

If removes are batched by the same item type (see new RemoveItemsByItemType(), the cost to build a HashSet lookup on-demand is trivial compared to the current approach of maintaining an additional O(1) lookup table with every dictionary entry - especially given that removes are only used in a few scenarios.

Perf

Working set memory at end of build (-17% and nearly the entire Large Object Heap):
Before
image

After
image

Total allocations (-300MB, finally no longer double digits!):
Before
image

After
image

This one also has a pretty significant drop in total GC time relative to allocations, I'm guessing due to the difference in LOH? I compared 4 profiles here to make sure this wasn't noise.

Before
image

After
image

This also further reduces CPU when merging down to the base Scope. Much of the remaining cost here was related to updating the additional dictionary, and this is also the only location where batch removes occur.

Before
image

After
image

@ccastanedaucf ccastanedaucf marked this pull request as ready for review August 23, 2025 01:21
Copilot AI review requested due to automatic review settings August 23, 2025 01:21
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes LinkedList usage from ItemDictionary and replaces it with a simpler List-based implementation for significant performance improvements. The change eliminates the O(1) LinkedListNode lookup table that was maintained for every dictionary entry, reducing memory allocations and garbage collection pressure.

Key Changes

  • Replaced LinkedList with List in ItemDictionary for better performance
  • Removed several unused ItemDictionary APIs (Contains, AddRange, Replace, etc.)
  • Introduced batch-based remove operations that build HashSets on-demand
  • Added ItemDictionarySlim for internal scope management without locking overhead

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
ImmutableItemDictionary.cs Removed unused API methods and simplified interface
ItemDictionary.cs Core refactor from LinkedList to List with optimized remove operations
IItemDictionary.cs Updated interface to remove unused methods and add batch remove
TargetUpToDateChecker.cs Updated to use ImportItemsOfType instead of AddEmptyMarker
TargetEntry.cs Added TruncateLookupsForItemTypes calls for proper scope handling
Lookup.cs Major refactor with new ItemDictionarySlim and optimized item resolution
ItemBucket.cs Updated to use TruncateLookupsForItemTypes instead of PopulateWithItems
ItemGroupIntrinsicTask.cs Updated Remove call to use new batch API
BatchingEngine.cs Updated to use FrozenSet for item names
Test files Updated test code to match new APIs
Comments suppressed due to low confidence (1)

src/Build/Collections/ItemDictionary.cs:1

  • The linear search through the list for removing a single item creates O(n) performance for removes. Since this is a hot path and performance is critical, consider using a Dictionary<T, int> index lookup table for O(1) removes, or document that single removes are expected to be rare compared to batch removes.
// Licensed to the .NET Foundation under one or more agreements.

@ghost ghost assigned surayya-MS Sep 2, 2025
@ccastanedaucf ccastanedaucf force-pushed the dev/chcasta/perf-itemdictionary-nolinkedlist branch from d60e258 to cc0b9e4 Compare October 1, 2025 04:19
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.

4 participants