Reduce heap size of RegisteredTaskRecord

[Erarndt]

Fixes #

Context

Part of GC cost is the amount of surviving objects during a collection. During solution load, this stack showed up as currently rooted items:

You can see that there are roughly 30MB worth of object instances being held onto by the tables collection inside of ConcurrentDictionary.

Additionally, ConcurrentDictionary objects are significantly larger than a Dictionary. For instance:

10k of them when empty:

10k of them with one item:

This is a size difference between 5x and 15x.

The existing implementation can be swapped to just use a dictionary with a lock.

Changes Made

Testing

Notes

[Copilot]

Pull Request Overview

This PR optimizes memory usage by replacing ConcurrentDictionary with a regular Dictionary protected by locks in the RegisteredTaskRecord class. The change addresses heap pressure during solution load where ConcurrentDictionary objects were consuming significant memory (30MB+ of surviving objects).

• Replaces ConcurrentDictionary<RegisteredTaskIdentity, object> with Dictionary<RegisteredTaskIdentity, object>
• Implements manual locking around dictionary access to maintain thread safety
• Reduces memory overhead by 5x-15x based on profiling data

Comments suppressed due to low confidence (1)

src/Build/Instance/TaskRegistry.cs:1441

• The finally block is missing a closing brace. There should be two closing braces - one for the lock block and one for the finally block.

                        _taskNamesCreatableByFactory[taskIdentity] = creatableByFactory;

[YuliiaKovalova]

@surayya-MS heads-up, it's related to your recent changes

[surayya-MS]

Thanks for the PR! There are definitely reduced memory optimizations here.

However, I'm still leaning towards using ConcurrentDictionary instead of normal Dictionary + lock for the new Multi-threaded MSBuild feature because of optimized concurrent reads and writes.

@rainersigwald what do you think?

[Erarndt]

Thanks for the PR! There are definitely reduced memory optimizations here.

However, I'm still leaning towards using ConcurrentDictionary instead of normal Dictionary + lock for the new Multi-threaded MSBuild feature because of optimized concurrent reads and writes.

@rainersigwald what do you think?

I'd be curious how much contention you're actually seeing in these paths. From the heap dump, there are 41,000 RegisteredTaskRecord objects, so I don't expect there are significant contention issues.

The other thing to weigh with respect to performance in the Multi-threaded feature is that garbage collections are going to be even more impactful to performance. Just these ConcurrentDictionaries represent 6.3% of all object alive in the heap in the snapshot. There is a cost being paid due to the initial allocations (that cause GC to trigger more frequently) and the memory being kept alive (promoting more memory makes a GC take more time). Each GC pauses all managed threads, so any savings here can have a significant impact.

[rainersigwald]

I think my big question here is "why are there enough of these RegisteredTaskRecords to matter?"

The set of tasks is pretty uniform between projects--should we tackle the size here by refactoring to share a single task registration object per resolved task, so e.g. in normal cases there's only one Csc? Is it per-project now?

[rainersigwald]

This change looks fine to me for now/if we don't pursue the better refactoring.