Skip to content

Use cached StringBuilder in CreateManifestResourceName#7012

Merged
rokonec merged 1 commit into
dotnet:mainfrom
ladipro:use-sb-cache-in-create-manifest-name
Nov 5, 2021
Merged

Use cached StringBuilder in CreateManifestResourceName#7012
rokonec merged 1 commit into
dotnet:mainfrom
ladipro:use-sb-cache-in-create-manifest-name

Conversation

@ladipro
Copy link
Copy Markdown
Member

@ladipro ladipro commented Nov 3, 2021

Fixes #5551

Context

Addressing a small inefficiency pointed out in #5551. If the task is given multiple resource files it wastefully creates a new StringBuilder for each.

Changes Made

Reuse a cached StringBuilder when creating resource names.

Testing

Existing unit tests.

Notes

  • The StringBuilder is not used "append-only" so this is not a good fit for SpanBasedStringBuilder.
  • I did not put the release call inside a finally block because if the code throws an exception "leaking" a StringBuilder is not a big deal with very minor perf impact compared to the exception handling machinery.

@Forgind Forgind added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Nov 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Performance merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Further improve CreateManifestResourceName memory allocations

5 participants