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

filter references for satellite assembly generation down to just the primary assemblies #38504

Merged
merged 7 commits into from
Aug 7, 2024

Conversation

baronfel
Copy link
Member

@baronfel baronfel commented Feb 2, 2024

Fixes #38478.

Satellite assemblies have a very constrained dependency list - essentially just the 'primary assembly' for the runtime platform being compiled for. This means one of mscorlib, netstandard, or System.Runtime. We currently pass the entire set of reference assemblies to the Csc Task when generating satellite assemblies, which means that the Roslyn compiler has to spend quite a bit of time on reference metadata reads, often evicting other more useful assemblies from the Roslyn compiler server's caches.

We can take advantage of this constrained generation domain to limit the set of references passed to the compiler invocation to just the 'primary assemblies', which should save quite a lot of time in aggregate.

Results

On my machine, for each 'cluster' of CSC invocations for satellite assemblies during the Roslyn build, the first call took ~800ms, and subsequent calls took ~75ms. After this change, the numbers are ~6ms and ~4ms respectively.

Aggregated across a single run of the Roslyn solution on my machine, I get

Name Total CoreGenerateSatelliteAssemblies Duration (s)
Before 107.2
After 66.0
quick F# Interactive script to grab out the duration of satellite assembly generation from a before.binlog and after.binlog file in the current directory
#r "nuget: MSBuild.StructuredLogger"

open Microsoft.Build.Logging.StructuredLogger
let beforeLog = Serialization.Read("./before.binlog")
let afterLog = Serialization.Read("./after.binlog")

let getCompilationTimes (b: Build) =
    let satelliteCalls = ResizeArray<_>()

    b.VisitAllChildren<Target> (fun t ->
        if t.Name = "CoreGenerateSatelliteAssemblies" then
            satelliteCalls.Add(t.Duration))

    satelliteCalls
    |> Seq.sumBy (fun d -> d.TotalSeconds)

printfn "Before: %A" (getCompilationTimes beforeLog)
printfn "After: %A" (getCompilationTimes afterLog)

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-ILLink untriaged Request triage from a team member labels Feb 2, 2024
@baronfel
Copy link
Member Author

To test this I need to be able to assert on the values of Items, which the current test framework doesn't support. #38811 tracks expanding the framework to support that. Will come back to this when I can green it up.

@jaredpar
Copy link
Member

jaredpar commented Jun 5, 2024

@davidwrighton and @agocke assured me that this approach is runtime approved. What do we need to do in order to hit the submit button here?

@baronfel
Copy link
Member Author

baronfel commented Jun 5, 2024

It needs to be cleaned up to not leak like @KalleOlaviNiemitalo mentioned, and it needs tests. My main problem is I have a bunch of other work that needs doing, so I probably need to get someone from the team to help get this over the line. @dotnet/domestic-cat maybe this would be a great one to focus on in-between codeflow/triage? Also needs rebasing onto 8.0.4xx.

@baronfel baronfel marked this pull request as ready for review August 6, 2024 15:03
@baronfel baronfel force-pushed the filter-satellite-assembly-refs branch from b7829fe to a4aa210 Compare August 6, 2024 15:24
@baronfel baronfel changed the base branch from release/8.0.3xx to main August 6, 2024 15:25
@baronfel baronfel enabled auto-merge August 6, 2024 21:11
@baronfel baronfel merged commit c4180ed into dotnet:main Aug 7, 2024
37 checks passed
@baronfel baronfel deleted the filter-satellite-assembly-refs branch August 7, 2024 21:56
@baronfel
Copy link
Member Author

baronfel commented Aug 9, 2024

Quick analysis from an actual Roslyn insertion:

Leg main branch this pr raw delta percent delta
Windows_Debug_Build 16:56 12:48 4:08 24.4%
Windows_Release_Build 16:43 14:29 2:14 13.4%
Unix_Build 7:34 7:09 0:24 5.5%
Source_Build 7:34 7:47 -0:13 -2.9%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-NetSDK untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generating satellite assemblies causes a lot of unnecessary work
6 participants