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

Improving sdk install performance #13128

Closed
sebastienros opened this issue Apr 12, 2023 · 3 comments · Fixed by #13359
Closed

Improving sdk install performance #13128

sebastienros opened this issue Apr 12, 2023 · 3 comments · Fixed by #13359

Comments

@sebastienros
Copy link
Member

I noticed that every time I would run build.cmd it is taking an unreasonable amount of time to detect that I already have the required runtimes locally in .\.dotnet. I managed to improve this time by using this check in tools.ps1:

function InstallDotNet([string] $dotnetRoot,
  [string] $version,
  [string] $architecture = '',
  [string] $runtime = '',
  [bool] $skipNonVersionedFiles = $false,
  [string] $runtimeSourceFeed = '',
  [string] $runtimeSourceFeedKey = '',
  [switch] $noPath) {

  # BEGIN
  $runtimePath = $dotnetRoot

  $runtimePath = $runtimePath + "\shared"
  if ($runtime -eq "dotnet") { $runtimePath = $runtimePath + "\Microsoft.NETCore.App" }
  if ($runtime -eq "aspnetcore") { $runtimePath = $runtimePath + "\Microsoft.AspNetCore.App" }
  $runtimePath = $runtimePath + "\" + $version

  if (Test-Path $runtimePath) {
    $installSuccess = $true
    Exit
  }
  # END

  $installScript = GetDotNetInstallScript $dotnetRoot
  $installParameters = @{
    Version = $version

I believe that just loading dotnet-install.ps1 is already costly, and on repositories that target multiple runtimes it is accumulating quickly. On my previous machine with aspnetcore it would spend 10s for nothing on each build.

But I think this could be improved even more if the calls to dotnet-install.cmd were skipped altogether and that would performance even better since it would prevent from starting a cmd which itself calls powershell then loads the scripts, for each runtime in global.json. My solution is barely skipping the dotnet-install.ps1 invocation.

@markwilkie
Copy link
Member

I'd be open to a PR on this @sebastienros.

@sebastienros
Copy link
Member Author

I am glad you are suggesting it as I didn't want to start anything if you had reasons not do accept changes around that. Thanks

@RussKie
Copy link
Member

RussKie commented Apr 17, 2023

This has been bothering me too. It's both too time consuming and too noisy. I have already made changes to dotnet-install scripts to reduce the chattiness.

I see there are possibly few ways of achieving the improvement. Probably the easiest option is to add a new switch - e.g., --skip-restore-toolset | -srt - that would bypass this command:

InstallDotNet $installdir $version $architecture $runtime $true -RuntimeSourceFeed $RuntimeSourceFeed -RuntimeSourceFeedKey $RuntimeSourceFeedKey

This way for the first time a dev will need to run .\restore.cmd to install the required toolsets, and then run .\restore.cmd -srt to only install the SDK, if required.

A more hands-off kind of solution could be building the runtime-pack paths and then conditionally run the above command.

RussKie added a commit that referenced this issue May 1, 2023
Check if runtime toolsets specfied in global.json under tools/runtimes
node exist before attempting to restore those. This check removes
unnecessary wait time and reduces the console "noise".

Resolves #13128
RussKie added a commit that referenced this issue May 1, 2023
Check if runtime toolsets specfied in global.json under tools/runtimes
node exist before attempting to restore those. This check removes
unnecessary wait time and reduces the console "noise".

Resolves #13128
markwilkie pushed a commit that referenced this issue May 5, 2023
Check if runtime toolsets specfied in global.json under tools/runtimes
node exist before attempting to restore those. This check removes
unnecessary wait time and reduces the console "noise".

Resolves #13128
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants