Skip to content
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

ExpandItemVectorIntoString interns string which gets on the large object heap #2678

Closed
Tracked by #6940
cdmihai opened this issue Oct 27, 2017 · 3 comments
Closed
Tracked by #6940
Assignees
Labels
backlog performance Performance-Scenario-Build This issue affects build performance. Performance-Scenario-Solution-Open This issue affects solution open performance. Priority:2 Work that is important, but not critical for the release size:1 triaged
Milestone

Comments

@cdmihai
Copy link
Contributor

cdmihai commented Oct 27, 2017

Measured on Roslyn rebuild (via msbuild /v:m /clp:PerformanceSummary;Summary Roslyn.sln /m:1)

image

Investigation is needed on why the method is interning the string. Is there any chance of the string getting reused? Is the string value unique?

@AndyGerlicher AndyGerlicher added this to the MSBuild 15.6 milestone Nov 6, 2017
@panopticoncentral panopticoncentral added Performance-Scenario-Solution-Open This issue affects solution open performance. Performance-Scenario-Build This issue affects build performance. labels Jun 9, 2020
@panopticoncentral panopticoncentral removed this from the MSBuild 15.6 milestone Jun 9, 2020
@panopticoncentral panopticoncentral added the Priority:2 Work that is important, but not critical for the release label Mar 23, 2021
@ladipro ladipro added the size:1 label Oct 12, 2021
@ladipro
Copy link
Member

ladipro commented Oct 12, 2021

Labeling with size:1 as the cost of the initial investigation. Will open a follow-up issue if more work is identified.
Note: This is an old issue, the code may have changed significantly.

@AR-May
Copy link
Member

AR-May commented Nov 25, 2021

I have checked ExpandItemVectorsIntoString function.

  1. The code has changed much. Now instead ReusableStringBuilder we use SpanBasedStringBuilder from StringTools.
  2. According to my measurements, most of the strings created in this function are small enough and do not get into LOH. But sometimes we get big string enough to get there.
  3. Even the long strings are created in this function not only once. So, they are not unique even in the scope of this function.

I think current behavior is reasonable enough.

However, when I looked at the trace I found one other place where ReusableStringBuilder is used and which we might want to optimize: #7086

@AR-May AR-May moved this from In Progress to Done in Perf sprint Nov 22nd - Dec 5th 2021 Nov 26, 2021
@AR-May
Copy link
Member

AR-May commented Nov 29, 2021

I am closing this issue. However, please feel free to reopen it if you want further discussion.

@AR-May AR-May closed this as completed Nov 29, 2021
@ladipro ladipro added this to the VS 17.1 milestone Dec 8, 2021
@AR-May AR-May added the triaged label Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog performance Performance-Scenario-Build This issue affects build performance. Performance-Scenario-Solution-Open This issue affects solution open performance. Priority:2 Work that is important, but not critical for the release size:1 triaged
Development

No branches or pull requests

6 participants