Avoid doing unncecessary work when generating component declaration files #24445

pranavkm · 2020-07-30T16:16:51Z

The output of the declaration file for Razor components are unaffected by all inputs other than the input .razor file.
Consequently we can avoid regenerating these files if the output is newer than the input. This is the same heuristic we apply to Blazor WebAsssembly's
compression artifacts.

This PR combines these two improvements for a ~90ms (10%) improvement in the inner loop.

       17 ms  GenerateBlazorWebAssemblyBootJson          1 calls
       22 ms  Copy                                       8 calls
       39 ms  ProcessFrameworkReferences                 1 calls
       40 ms  RazorTagHelper                             1 calls
       51 ms  ResolveAssemblyReference                   1 calls
       70 ms  GetFileHash                                1 calls
       80 ms  RazorGenerate                              2 calls
      111 ms  Csc                                        2 calls

      Time Elapsed 00:00:00.95

       17 ms  GenerateBlazorWebAssemblyBootJson          1 calls
       21 ms  Copy                                       8 calls
       37 ms  ProcessFrameworkReferences                 1 calls
       51 ms  ResolveAssemblyReference                   1 calls
       70 ms  Csc                                        1 calls
       72 ms  GetFileHash                                1 calls
       79 ms  RazorGenerate                              2 calls

Time Elapsed 00:00:00.86

In after: Csc calls reduced to one, RazorTagHelper call removed.

Contributes to #22566

…iles. The output of the declaration file for Razor components are unaffected by all inputs other than the input .razor file. Consequently we can avoid regenerating these files if the output is newer than the input. This is the same heuristic we apply to Blazor WebAsssembly's compression artifacts. This PR combines these two improvements for a ~90ms (10%) improvement in the inner loop. ``` 17 ms GenerateBlazorWebAssemblyBootJson 1 calls 22 ms Copy 8 calls 39 ms ProcessFrameworkReferences 1 calls 40 ms RazorTagHelper 1 calls 51 ms ResolveAssemblyReference 1 calls 70 ms GetFileHash 1 calls 80 ms RazorGenerate 2 calls 111 ms Csc 2 calls Time Elapsed 00:00:00.95 ``` ``` 17 ms GenerateBlazorWebAssemblyBootJson 1 calls 21 ms Copy 8 calls 37 ms ProcessFrameworkReferences 1 calls 51 ms ResolveAssemblyReference 1 calls 70 ms Csc 1 calls 72 ms GetFileHash 1 calls 79 ms RazorGenerate 2 calls Time Elapsed 00:00:00.86 ``` In after: Csc calls reduced to one, RazorTagHelper call removed.

pranavkm · 2020-07-30T17:20:53Z

src/Razor/Microsoft.AspNetCore.Razor.Tools/src/GenerateCommand.cs

@@ -201,6 +201,7 @@ protected override bool ValidateArguments()
                if (GenerateDeclaration.HasValue())
                {
                    b.Features.Add(new SetSuppressPrimaryMethodBodyOptionFeature());
+                    b.Features.Add(new SuppressChecksumOptionsFeature());


This prevents the pragma checksum from being generated on declaration files. The checksum is used by debugger. Declaration files aren't used as part of debugging, they're only used for component discovery. Having the checksum is actively harmful since any changes to the input would result in the checksum being different.

pranavkm · 2020-07-30T17:23:36Z

...r/Microsoft.NET.Sdk.Razor/src/build/netstandard2.0/Microsoft.NET.Sdk.Razor.Component.targets

@@ -85,7 +86,7 @@ Copyright (c) .NET Foundation. All rights reserved.
    Name="RazorGenerateComponentDeclaration"
    DependsOnTargets="$(RazorGenerateComponentDeclarationDependsOn)"
    Inputs="$(MSBuildAllProjects);@(RazorComponentWithTargetPath);$(_RazorComponentInputCacheFile)"
-    Outputs="@(_RazorComponentDeclaration)"
+    Outputs="$(_RazorComponentDeclarationOutputCacheFile)"


Prior to this, we relied on the declaration files being older than the inputs to determine if the targets need to be executed. This now uses an explicit marker file. This allows declaration files to not be updated, but for us to track if we at least tried regenerating the declaration file.

What do the contents of this declaration file look like? Does the RazorGenerateComponentDeclaration task need to be changed?

The declaration files are the outline of the components with out any body (they kinda look like ref assemblies). It lets us know what components exist and what parameters are on them. Prior to this change, if you were editing Index.razor to change some markup, here's what the sequence of events would look like:

RazorGenerateComponentDeclaration Inputs: Index.razor Output: obj\Debug\net5.0\RazorDeclaration\Index.razor.g.cs MSBuild would evaluate this target because `Index.razor` is newer than `Index.razor.g.cs` (since it was just edited). At the end of this target, `Index.razor.g.cs` would have a new timestamp but the contents would be unchanged. Next up is the compile and tag helper discovery targets that we use to find out what changed in the compnents: RazorCompileComponentDeclaration Inputs: obj\Debug\net5.0\RazorDeclaration\Index.razor.g.cs Output: obj\Debug\net5.0\RazorDeclaration\Assembly.declaration.dll Since `Index.razor.g.cs` is newer, MSBuild will evaluate this target and perform CSC. Next up, since the declaration assembly is newer, we'll perform a tag helper discovery on it.

RazorGenerateComponentDeclaration Inputs: Index.razor Output: SomeMarkerFile MSBuild would evaluate this target because `Index.razor` is newer than `SomeMarkerFile`. At the end of this target, `Index.razor.g.cs` would have not a have newer timestamp. This allows the next two targets to be skipped. SomeMarkerFile helps us track if the target was evaluated since the inputs were last changed.

captainsafia · 2020-07-30T19:12:57Z

src/Razor/Microsoft.AspNetCore.Razor.Tools/src/GenerateCommand.cs

@@ -227,6 +228,7 @@ protected override bool ValidateArguments()
            });

            var results = GenerateCode(engine, sourceItems);
+            var isGeneratingDeclaration = GenerateDeclaration.HasValue();


Is there a particular reason the GenerateDeclaration.HasValue is cached here instead of called in the if-statement?

It does a linq Any() call. It seems unnecessary to perform n-times (once for each code file we're code-generating).

ghost · 2020-07-31T17:31:46Z

Hello @pranavkm!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

…iles. (#24445) The output of the declaration file for Razor components are unaffected by all inputs other than the input .razor file. Consequently we can avoid regenerating these files if the output is newer than the input. This is the same heuristic we apply to Blazor WebAsssembly's compression artifacts. This PR combines these two improvements for a ~90ms (10%) improvement in the inner loop. ``` 17 ms GenerateBlazorWebAssemblyBootJson 1 calls 22 ms Copy 8 calls 39 ms ProcessFrameworkReferences 1 calls 40 ms RazorTagHelper 1 calls 51 ms ResolveAssemblyReference 1 calls 70 ms GetFileHash 1 calls 80 ms RazorGenerate 2 calls 111 ms Csc 2 calls Time Elapsed 00:00:00.95 ``` ``` 17 ms GenerateBlazorWebAssemblyBootJson 1 calls 21 ms Copy 8 calls 37 ms ProcessFrameworkReferences 1 calls 51 ms ResolveAssemblyReference 1 calls 70 ms Csc 1 calls 72 ms GetFileHash 1 calls 79 ms RazorGenerate 2 calls Time Elapsed 00:00:00.86 ``` In after: Csc calls reduced to one, RazorTagHelper call removed.

…iles. (dotnet/aspnetcore#24445) The output of the declaration file for Razor components are unaffected by all inputs other than the input .razor file. Consequently we can avoid regenerating these files if the output is newer than the input. This is the same heuristic we apply to Blazor WebAsssembly's compression artifacts. This PR combines these two improvements for a ~90ms (10%) improvement in the inner loop. ``` 17 ms GenerateBlazorWebAssemblyBootJson 1 calls 22 ms Copy 8 calls 39 ms ProcessFrameworkReferences 1 calls 40 ms RazorTagHelper 1 calls 51 ms ResolveAssemblyReference 1 calls 70 ms GetFileHash 1 calls 80 ms RazorGenerate 2 calls 111 ms Csc 2 calls Time Elapsed 00:00:00.95 ``` ``` 17 ms GenerateBlazorWebAssemblyBootJson 1 calls 21 ms Copy 8 calls 37 ms ProcessFrameworkReferences 1 calls 51 ms ResolveAssemblyReference 1 calls 70 ms Csc 1 calls 72 ms GetFileHash 1 calls 79 ms RazorGenerate 2 calls Time Elapsed 00:00:00.86 ``` In after: Csc calls reduced to one, RazorTagHelper call removed. Commit migrated from dotnet/aspnetcore@f495fcb151e9

ghost added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Jul 30, 2020

pranavkm force-pushed the prkrishn/sdk-improvements branch from 8053398 to e1ce923 Compare July 30, 2020 16:18

pranavkm added area-blazor Includes: Blazor, Razor Components and removed area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates labels Jul 30, 2020

pranavkm force-pushed the prkrishn/sdk-improvements branch from e1ce923 to 91363ef Compare July 30, 2020 17:17

pranavkm requested a review from a team July 30, 2020 17:18

pranavkm added this to the 5.0.0-rc1 milestone Jul 30, 2020

pranavkm force-pushed the prkrishn/sdk-improvements branch from 91363ef to 131f796 Compare July 30, 2020 17:21

pranavkm commented Jul 30, 2020

View reviewed changes

captainsafia reviewed Jul 30, 2020

View reviewed changes

pranavkm requested a review from a team July 31, 2020 17:03

pranavkm added the auto-merge label Jul 31, 2020

captainsafia approved these changes Jul 31, 2020

View reviewed changes

ghost merged commit f495fcb into master Jul 31, 2020

ghost deleted the prkrishn/sdk-improvements branch July 31, 2020 20:40

jaredpar mentioned this pull request Aug 3, 2020

OSX machines are de-provisioned during CI / PR runs leading to failures dotnet/runtime#34472

Closed

pranavkm mentioned this pull request Aug 17, 2020

Build performance improvements for Blazor #22566

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avoid doing unncecessary work when generating component declaration files #24445

Avoid doing unncecessary work when generating component declaration files #24445

pranavkm commented Jul 30, 2020 •

edited

Loading

pranavkm Jul 30, 2020

pranavkm Jul 30, 2020

captainsafia Jul 30, 2020

pranavkm Jul 31, 2020

captainsafia Jul 30, 2020

pranavkm Jul 31, 2020

ghost commented Jul 31, 2020

Avoid doing unncecessary work when generating component declaration files #24445

Avoid doing unncecessary work when generating component declaration files #24445

Conversation

pranavkm commented Jul 30, 2020 • edited Loading

pranavkm Jul 30, 2020

Choose a reason for hiding this comment

pranavkm Jul 30, 2020

Choose a reason for hiding this comment

captainsafia Jul 30, 2020

Choose a reason for hiding this comment

pranavkm Jul 31, 2020

Choose a reason for hiding this comment

captainsafia Jul 30, 2020

Choose a reason for hiding this comment

pranavkm Jul 31, 2020

Choose a reason for hiding this comment

ghost commented Jul 31, 2020

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

pranavkm commented Jul 30, 2020 •

edited

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.