Skip to content

Conversation

@pranavkm
Copy link
Contributor

  • In the ordinary case, diagnostics aren't produced by builders. The current pattern results in allocating a HashSet
    and an empty array for this case. Removing some of these allocations
  • ViewComponentTagHelperPass does not need to operate on .razor files, but does some unnecessary work. We can avoid it

@pranavkm pranavkm requested a review from Pilchie as a code owner August 16, 2021 20:18
@ghost ghost added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Aug 16, 2021
@pranavkm pranavkm requested review from a team, NTaylorMullen and captainsafia August 16, 2021 20:18

protected override void ExecuteCore(RazorCodeDocument codeDocument, DocumentIntermediateNode documentNode)
{
if (FileKinds.IsComponent(codeDocument.GetFileKind()))
Copy link
Member

Choose a reason for hiding this comment

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

Are there other passes we can apply this heuristic on? E.g. ModelExpressionPass.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it does look like we could do more here. This one popped up when I was debugging / profiling.

* [x] In the ordinary case, diagnostics aren't produced by builders. The current pattern results in allocating a HashSet
and an empty array for this case. Removing some of these allocations
* [x] ViewComponentTagHelperPass does not need to operate on .razor files, but does some unnecessary work. We can avoid it
@pranavkm pranavkm force-pushed the prkrishn/some-allocations branch from 6e06b1f to 1ff0978 Compare August 16, 2021 22:18

internal static bool IsInvalidNonWhitespaceHtmlCharacters(char testChar)
{
foreach (var c in InvalidNonWhitespaceHtmlCharacters)

Choose a reason for hiding this comment

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

Does this not allocate an enumerator? Wondering why not a for loop is all 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

oh wow, so the compiler special cases arrays or something?

Copy link
Member

Choose a reason for hiding this comment

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

the compiler special cases arrays

It's not dictated by the language specifications, but as an optional optimization .NET's C# compiler (Roslyn) special cases (single dimensional) arrays, and strings so code equivalent to a for-loop is emitted.

@pranavkm pranavkm enabled auto-merge (squash) August 16, 2021 23:17
@pranavkm pranavkm merged commit e1aeac8 into main Aug 16, 2021
@pranavkm pranavkm deleted the prkrishn/some-allocations branch August 16, 2021 23:57
@ghost ghost added this to the 6.0-rc1 milestone Aug 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants