Skip to content

Avoid boxing allocations for creation of error message in non-error cases - #12537

Merged
SimaTian merged 2 commits into
dotnet:mainfrom
Erarndt:dev/erarndt/reportResultBox
Sep 24, 2025
Merged

SimaTian merged 2 commits into
dotnet:mainfrom
Erarndt:dev/erarndt/reportResultBox

Conversation

@Erarndt

@Erarndt Erarndt commented Sep 16, 2025

Copy link
Copy Markdown
Contributor

Fixes #

Context

With the current way the code is constructed, the parameters for the error message in ReportResult() end up getting boxed even if they never end up getting used. By expanding this a little and guarding the call based on the error condition, we only pay the allocation cost in the error state.

There are ~11MB of allocations due to the types being boxed.
image

Changes Made

Testing

Notes

Copilot AI review requested due to automatic review settings September 16, 2025 21:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 optimizes performance by avoiding unnecessary boxing allocations when creating error messages in non-error scenarios. The change refactors a single line to conditionally format error messages only when the error condition is actually met, eliminating ~11MB of allocations from boxing the message parameters.

Key Changes

  • Replaced ErrorUtilities.VerifyThrow with manual condition checking followed by ErrorUtilities.ThrowInternalError
  • Added guard condition to only format error message parameters when an error actually occurs

Comment thread src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEntry.cs Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@SimaTian
SimaTian merged commit 5480b47 into dotnet:main Sep 24, 2025
9 checks passed
@Erarndt
Erarndt deleted the dev/erarndt/reportResultBox branch September 24, 2025 18:33
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