Skip to content

Conversation

@MichalPavlik
Copy link
Member

Reverts the revert of #12510 :) Original PR #12249

Copilot AI review requested due to automatic review settings September 11, 2025 14:13
Copy link
Member

@JanProvaznik JanProvaznik left a comment

Choose a reason for hiding this comment

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

the regression turned out to be unrelated despite having similar names to concepts changed in this pr

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 reverts the revert of memory allocation optimizations when creating ProjectInstance from immutable project cache state. The primary purpose is to reduce memory usage during project instance creation by introducing lazy-loading patterns and more efficient collection converters.

Key changes include:

  • Introduces LazyStringValuedList for deferred string list initialization
  • Implements ImmutableProjectMetadataCollectionConverter and ImmutableProjectPropertyCollectionConverter for optimized collection handling
  • Updates TranslatorHelpers.cs to support IReadOnlyDictionary<string, string> parameters

Reviewed Changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
src/Shared/TranslatorHelpers.cs Adds new overload for translating IReadOnlyDictionary<string, string> and refactors existing ImmutableDictionary method to use the new implementation
src/Build/Microsoft.Build.csproj Updates project file to include new collection converter classes and removes old converter implementations
src/Build/Instance/ProjectItemInstance.cs Changes metadata storage from ImmutableDictionary to IReadOnlyDictionary with lazy conversion and optimized property access
src/Build/Instance/ProjectInstance.cs Implements lazy import path loading and optimized property collection handling for immutable project sources
src/Build/Instance/ImmutableProjectCollections/LazyStringValuedList.cs New class implementing lazy-initialized, immutable string list for import paths
src/Build/Instance/ImmutableProjectCollections/ImmutableProjectPropertyCollectionConverter.cs New converter for optimized property collection handling with direct value retrieval
src/Build/Instance/ImmutableProjectCollections/ImmutableProjectMetadataCollectionConverter.cs New converter for metadata collections with lazy dictionary conversion
src/Build/Instance/ImmutableProjectCollections/ImmutableGlobalPropertiesCollectionConverter.cs Enhanced with IValueDictionaryConverter interface support
src/Build/Collections/RetrievableEntryHashSet/IRetrievableUnescapedValuedEntryHashSet.cs New interface for direct unescaped value retrieval without object allocation
src/Build/Collections/PropertyDictionary.cs Enhanced with unescaped value retrieval and read-only dictionary conversion
src/Build/Collections/IValueDictionaryConverter.cs New interface for specialized property dictionaries
Test files Multiple test files implementing mock objects for testing the new functionality

@MichalPavlik
Copy link
Member Author

the regression turned out to be unrelated despite having similar names to concepts changed in this pr

Yes, it was proved that these changes didn't caused the regression.

@MichalPavlik MichalPavlik enabled auto-merge (squash) September 11, 2025 14:51
@MichalPavlik MichalPavlik merged commit 610aa6f into main Sep 11, 2025
9 checks passed
@MichalPavlik MichalPavlik deleted the revert-12510-revert-12249-dev/lifengl/reduceImmutableProjectInstanceAllocation branch September 11, 2025 18:09
JanProvaznik added a commit that referenced this pull request Sep 16, 2025
…nce is created from immutable project cache state"" (#12513)"

This reverts commit 610aa6f.
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.

5 participants