search

dotnet / razor

Compiler and tooling experience for Razor ASP.NET Core apps in Visual Studio, Visual Studio for Mac, and VS Code.
https://asp.net
MIT License
474 stars 186 forks source link

Performance: SyntaxToken objects are too large #7056

Open ryanbrandenburg opened 3 years ago

ryanbrandenburg commented 3 years ago

From https://github.com/dotnet/aspnetcore/issues/30247#issuecomment-797162033.

An average SyntaxToken seems to weigh ~.1MB, with their total footprint in our completion scenario working out to ~305MB.

If I had to guess without looking at it I'd say most of that space was spent on strings which were Parsed in and which could probably be run through our StringCache. That only reduces how long they persist, but when parsing strings we're gonna allocate and we might get them collected in an earlier (cheaper) Gen. Should do actual analysis before acting though.

This also applies to MarkupTextLiteralSyntax, DescendantNodesImpl, InternalSyntax.SyntaxToken, MarkupTextLiteralSyntax, and SpanEditHandler, but depending on how related the cause/fix for each problem is you might want to split them out into their own issue.

ghost commented 3 years ago

Thanks for contacting us. We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

ryanbrandenburg commented 3 years ago

Compared to total VS allocations this isn't actually THAT bad. Therefore do a quick investigation, but if the fix isn't easy it's not worth it at this time.

NTaylorMullen commented 2 years ago

The goal here would be to investigate incremental parsing to make the allocations & compiler cpu bound work less

ghost commented 2 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

javiercn commented 2 years ago

Can you provide more details on this perf investigation so that we can make progress? We would need:

In addition to that, it would be important to include the GC statistics offered by perfview to help us determine if this is worth pursuing.

davkean commented 2 years ago

Compared to total VS allocations this isn't actually THAT bad. Therefore do a quick investigation, but if the fix isn't easy it's not worth it at this time.

This triggers my spider sense, if that was the attitude everywhere we wouldn't make progress anywhere. :)

@javiercn We have a variety of sources where we can pull some data. First, we have some perf tests (via our "Speedometer" effort) that sit over Orchard Core, a very heavy Razor user, I was looking at said trace when I stumbled on this issue. Secondly, we have live telemetry sources, both via Memory Watson (captures high memory dumps in the wild) and GC Pause Watson (captures live GC pause data when GC time is over 30%). On top of that there was this one time when VS accidently turned off Razor support one insertion, and we also have the results from reverting that change (see #1472882) which shows Razor/Razor tooling's direct impact on both responsiveness and GC pressure.

I've been meaning to sit down and get a handle on Razor's impact but the problem is that I lack context on the stacks and scenarios and would love to sit down with a Razor dev for a few hours with a couple of representative traces and then get actionable bugs filed from it. Is there a good person to work with on this?