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

Infra: Reduce build time to perform CLR up-to-date check #47022

Open
3 of 7 tasks
agocke opened this issue Jan 15, 2021 · 9 comments
Open
3 of 7 tasks

Infra: Reduce build time to perform CLR up-to-date check #47022

agocke opened this issue Jan 15, 2021 · 9 comments
Labels
area-Infrastructure-coreclr help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@agocke
Copy link
Member

agocke commented Jan 15, 2021

The time being measured is the time needed to build the clr subset, after the subset has been completely built already.

If this time were low enough, under 5 seconds or so, it would be feasible to add it as a dependency to other subsets. As it is, it takes roughly 51s on my machine.

Ongoing list of work items, to be updated until we're at 5 secons or less:

  • Incrementally crossgen System.Private.CoreLib
  • clretwrc appears to always be rebuilt
  • Switch to Ninja in Windows by default
    • This will require a workflow for building and preserving VS project files
  • Consider changing the default meaning of clr partition
    • Linux DAC is built by default on Windows (this is expensive. We could try to make it cheaper, or move it out of dev build)
    • CLR managed tools are built as part of CLR
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@hoyosjs
Copy link
Member

hoyosjs commented Feb 9, 2021

After building with #48044, a release build up-to-date-check using build-runtime.cmd writes no files other than the CMake cache.

Some rough numbers with the MSBuild/VS generator on my 8 core machine for an up-to-date check on release x64 fornative builds:

  • With ./build.cmd clr.runtime -rc release it takes somewhere between 40 and 50 seconds. There's a small overhead on restore of tools here.
  • With direct call into build-runtime (src/coreclr/build-runtime.cmd -release -x64 -enforcepgo) Roughly 40 seconds.
  • With direct call into build-runtime.cmd and skipping CMake setup (src/coreclr/build-runtime.cmd -release -x64 -enforcepgo -skipconfigure) 25-30 seconds. This option is not bubbled up to the root build script.
  • MSBuild takes 20s-25s of this.

With ninja:

  • root script and build-runtime (src/coreclr/build-runtime.cmd -release -x64 -enforcepgo) take roughly 18-20 seconds.
  • build-runtime.cmd and skipping CMake setup gives 10-12 seconds.

@hoyosjs
Copy link
Member

hoyosjs commented Feb 9, 2021

Largely 10 seconds are used in invoking MSBuild in 3 invocations:

  • Generate a version header.
  • Restore the pgo package
  • Generate a PGO path file.

These could arguably be munged into 1 (max two) calls.

@ViktorHofer
Copy link
Member

Largely 10 seconds are used in invoking MSBuild in 3 invocations:

Are those steps running in incremental builds as well? The version header could presumably have inputs and outputs applied. Restoring via nuget is already incremental so nothing to do there.

@trylek trylek added the help wanted [up-for-grabs] Good issue for external contributors label Mar 2, 2021
@agocke
Copy link
Member Author

agocke commented Mar 11, 2021

@hoyosjs Looking at the log, when I run build clr -ninja I still see

Read file version from native version header at 'C:/Users/andy/Code/runtime/src/coreclr/../../artifacts/obj/coreclr/_version.h'.
-- Configuring done
-- Generating done
-- Build files have been written to: C:/Users/andy/Code/runtime/artifacts/nmakeobj/windows.x64.Debug
[1/7] Linking RC shared library dlls\clretwrc\clretwrc.dll
[2/7] Linking RC shared library dlls\mscorrc\mscorrc.dll

I wasn't expecting us to have to re-link clretwrc. Any ideas?

@hoyosjs
Copy link
Member

hoyosjs commented Mar 11, 2021

Tracked it down I think. Let me test if it's that.

@hoyosjs
Copy link
Member

hoyosjs commented Mar 12, 2021

Hmm. Looks like what I was seeing will only affect non-Windows system. Will try to see where this is coming from.

@ericstj
Copy link
Member

ericstj commented Apr 7, 2022

Just imaged a brand new machine and was measuring build performance. On a repo with zero changes rebuild of CLR subset is taking over a minute and a half, while build of libs is about 20s.

It looks to me like CLR is rebuilding a bunch of stuff. I put up a log at \bvtsrv2\team\ericstj\incremental-build-clr

@hoyosjs
Copy link
Member

hoyosjs commented Apr 8, 2022

Looks like this took a 1:15
"C:\src\dotnet\runtime\src\coreclr\build-runtime.cmd" -x64 -release -pgodatapath "C:\Users\ericstj\.nuget\packages\optimization.windows_nt-x64.pgo.coreclr\1.0.0-prerelease.22121.2"

@agocke agocke modified the milestones: 7.0.0, Future Jul 26, 2022
sbomer added a commit that referenced this issue Jun 9, 2023
Before this change, the merged mibc file would inherit the
timestamp of the input (which are often days in the past, from
the optimization data nuget package). This caused merging to
happen on every incremental build, as dotnet-pgo.dll is always
newer than the output file.

The timestamp was inherited to fix a different incremental build
issue, where updating to a newer optimization data package
wouldn't make the input timestamp new enough to trigger a
re-merge. See #56397 for
context.

This fixes both issues by caching the input modification
timestamps, so that the merged output file will get a new
timestamp when it is created, but will be considered out of date
whenever the input timestamps change.

This reduces incremental build times on my machine by
~3s. Contributes to
#47022

Also deletes the --inherit-timestamp logic.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Infrastructure-coreclr help wanted [up-for-grabs] Good issue for external contributors
Projects
Status: No status
Development

No branches or pull requests

5 participants