Skip to content

Remove unused HashSet allocations in HashJavaNames#11124

Merged
jonathanpeppers merged 1 commit intomainfrom
jonathanpeppers/dev-peppers-remove-unused-hashsets-typem
Apr 16, 2026
Merged

Remove unused HashSet allocations in HashJavaNames#11124
jonathanpeppers merged 1 commit intomainfrom
jonathanpeppers/dev-peppers-remove-unused-hashsets-typem

Conversation

@jonathanpeppers
Copy link
Copy Markdown
Member

The hashes32 and hashes64 HashSet<T> instances in TypeMappingReleaseNativeAssemblyGenerator.HashJavaNames were populated every iteration but never read from -- pure wasted allocations during type map generation.

This was found while analyzing a binlog for GenerateTypeMappings task performance.

The hashes32 and hashes64 HashSet<T> instances were populated but
never read, resulting in unnecessary allocations during type map
generation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 16, 2026 14:14
Copy link
Copy Markdown
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 improves type map generation performance by removing unused per-call HashSet<T> allocations in TypeMappingReleaseNativeAssemblyGenerator.HashJavaNames, where the sets were populated but never read.

Changes:

  • Remove unused HashSet<uint> and HashSet<ulong> allocations from HashJavaNames.
  • Keep generating/storing both 32-bit and 64-bit Java name hashes on each TypeMapJava entry.

@jonathanpeppers
Copy link
Copy Markdown
Member Author

This change shouldn't affect anything, there are a couple flaky CI failures we can ignore:

Analysis
3 of 4 failures are performance tests (PerformanceTest.cs) that exceeded timing thresholds. These are commonly flaky on shared CI agents due to variable machine load. The margins were relatively small (e.g., 3268ms vs 3000ms threshold for Build_XAML_Change(False)).
1 failure is a Gradle binding test (BindLibraryWithMultipleGradleVersions) that failed during project build with AGP 8.5.0, Gradle 8.7, compileSdk 34. Unrelated to these changes.
Infrastructure issue: PowerShell crashed in MSBuild+Emulator 1 job post-test step with ManagedPSEntry+StartupException / procargs errno 5. This is an agent-level issue unrelated to the PR changes.

@jonathanpeppers jonathanpeppers merged commit 85f6331 into main Apr 16, 2026
9 of 10 checks passed
@jonathanpeppers jonathanpeppers deleted the jonathanpeppers/dev-peppers-remove-unused-hashsets-typem branch April 16, 2026 17:08
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