Skip to content

Reduce allocations in DependencyHelper.GetExtensionRequirements #11022

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

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

kshyju
Copy link
Member

@kshyju kshyju commented Apr 22, 2025

Optimized the following methods to reduce memory allocations and improve deserialization performance:

  1. DependencyHelper.GetExtensionRequirements()
  2. RuntimeAssembliesInfo.GetRuntimeAssemblies()
  3. RuntimeInformationProvider.BuildRuntimesGraph()

These methods previously used JObject.Parse to deserialize embedded JSON resources. This PR replaces them with System.Text.Json and source generation.

Before

Name Total (Allocations) Self (Allocations) Total Size (Bytes) Self Size (Bytes)
DependencyHelper.GetExtensionRequirements() 4,700 1 358,800 32
RuntimeAssembliesInfo.GetRuntimeAssemblies() 10,647 0 794,082 0

image

image

After

Name Total (Allocations) Self (Allocations) Total Size (Bytes) Self Size (Bytes)
DependencyHelper.GetExtensionRequirements() 420 12 31,548 384
RuntimeAssembliesInfo.GetRuntimeAssemblies() 1,432 0 101,568 0

image

image

Delta

  • DependencyHelper.GetExtensionRequirements – Total allocations dropped by 4,280 (91.06%)
  • DependencyHelper.GetExtensionRequirements – Total allocated size dropped by 319.5 KB (91.23%)
  • RuntimeAssembliesInfo.GetRuntimeAssemblies – Total allocations dropped by 9,215 (86.56%)
  • RuntimeAssembliesInfo.GetRuntimeAssemblies – Total allocated size dropped by 676.1 KB (87.21%)

Pull request checklist

IMPORTANT: Currently, changes must be backported to the in-proc branch to be included in Core Tools and non-Flex deployments.

  • Backporting to the in-proc branch is not required
    • Otherwise: Link to backporting PR
  • My changes do not require documentation changes
    • Otherwise: Documentation issue linked to PR
  • My changes should not be added to the release notes for the next release
    • Otherwise: I've added my notes to release_notes.md
  • My changes do not need to be backported to a previous version
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • My changes do not require diagnostic events changes
    • Otherwise: I have added/updated all related diagnostic events and their documentation (Documentation issue linked to PR)
  • [x ] I have added all required tests (Unit tests, E2E tests)

Additional information

Additional PR information

[Fact]
public void GetExtensionRequirementsReturnsEmbededManifestContent()
{
var extensionRequirements = DependencyHelper.GetExtensionRequirements();
Copy link
Member Author

Choose a reason for hiding this comment

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

Added a new test to ensure JSON deserialization works as expected.

@kshyju kshyju marked this pull request as ready for review April 22, 2025 22:24
@kshyju kshyju requested a review from a team as a code owner April 22, 2025 22:24
@kshyju kshyju requested a review from soninaren April 22, 2025 22:25
@kshyju kshyju force-pushed the shkr/getextensionreqs-improvements branch from 7faecfa to e5f0958 Compare May 6, 2025 02:32
@kshyju kshyju requested review from surgupta-msft and fabiocav May 6, 2025 14:09
@kshyju
Copy link
Member Author

kshyju commented May 6, 2025

@fabiocav @soninaren Could you please review these changes since you both have historical context on this area?

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.

2 participants