Fix for vcpkg run time issue in windows runner #4717
Conversation
Changes for msvc batch file invocation environment: * Add Powershell 7 to PATH * Add PSModulePath for Powershell 7 and 5 * Import VCPKG_DISABLE_METRICS if defined * Import VCPKG_ROOT if defined
|
@mwichmann The VS2017/PY37 environment is reporting that the MSVS 2015 license has expired. Image: Visual Studio 2017; Environment: WINPYTHON=Python37 Test failure: |
|
yes. @bdbaddog has contacted them about that (a couple of weeks ago, I think). they were supposed to be fixing. |
|
does this relate at all to #4226 ? |
I'm going to pretend like I understood the details of #4226 and answer confidently with "only tangentially?" My understanding of #4226, is the intent is to use vcpkg via the "standalone" vcpkg installation. The windows runner contains two
Note: it *may be possible to have the both installations use the same package root. This needs to be investigated. Additional VS components may put initialization batch files in the A simplified view of the VS directory contents for vcpkg is:
The initial call sequence is to setup the embedded VS vcpkg installation which is done using powershell and may download from the GitHub vcpkg repository. The embedded VS vcpkg batch files and powershell scripts run within the limited SCons msvc environment as it is kicked off from the msvc batch files. The windows runner environment has both powershell 7 and powershell 5 installed (different locations so side-by-side). The current SCons implementation adds powershell 5 to msvc environment PATH (i.e., In the windows runner, adding powershell 7 before powershell 5 in the msvc environment system path and adding a The first call tended to be more expensive than later calls. There was something about using powershell 5 without a PSModulePath that caused a problem. The environment in which vcpkg.exe was executed does not contain Windows system environment variables (e.g., ProgramFiles, ProgramData, PROCESSOR_ARCHITECTURE, etc). It is kind of difficult to debug the msvc batch file and powershell call chain as it is launched via a subprocess and there is limited diagnostic information available. It may be possible to use the "standalone" package root with the embedded vcpkg.exe by setting This PR passes along This PR is intended to be the minimum required to workaround the initialization penalty for the embedded vcpkg. I am planning on trying a few things locally. Unfortunately, a local instance is not ephemeral like the runner image which makes alternate configurations and testing a bit more work. Clear as mud? |
|
So is the summary of the summary that things don't work well if |
|
Vcpkg isn't even being "used" it was simply installed as a component of Visual Studio. The vcvars64.bat batch file is initializing the vcpkg component environment and experiences delays due to the aggressively limited environment in which the msvc batch files are run. I believe that the in-tree vcpkg data after initialization is lost when the runner completes so there is a runtime penalty every time the runner is invoked and to a lesser extend any subsequence invocations in the same session. The persistent SCons MSVC cache in the windows runner environment limits the penalty to the first invocation of each combination of msvc batch file and arguments. If the msvc cache is not persisted between runs, the runtime penalty applies every time. What I can't answer is:
|
|
My last question crossed paths, nevermind. A fully executed "Developer Command Prompt" (that is, all the setup scripts executed) has a LOT of extra variables, which we knew, we were largely guessing at the minimal set. Certainly has a There's a |
|
There are two minimal sets of variables:
The issue at hand is the set of variables going in and not necessarily the set coming out. In a perfect world, the set of variables kept from the SCons batch file should match exactly the same set of variable values when run from the command-line. However, due to the lack of inclusion of some Windows system variables that is not always the case. There are known msvc batch scripts that run every time but do not run correctly configure locations due to the lack of Windows system variables (e.g., PROCESSOR_ARCHITECTURE, PROGRAMFILES, etc). The vcpkg component is different in that it requires powershell to run which has exposed a fundamental difference between the system environment and the environment in which the msvc batch files are invoked. Aside: The msvc batch files invoke vswhere but it returns no output because ProgramData is not passed into the environment. That is why the banner in the SCons stdout always shows a short version (e.g., v17.0) rather than the full version number when run from the shell environment. There are days I'm astonished that the msvc batch files work at all within the limited environment. P.S.: I believe there is a bug in one of msvc batch files (perf_tools.bat). A registry search is looking for a 2019 subkey instead of a 2022 subkey. With only 2022 installed, there is no 2019 subkey. Oof! |
|
@jcbrill - so it looks like the slowdown is due to vcpkg being initialized by vcvarsall.bat (or equivalent)? Is there a flag to skip that initialization? (
Not surprising with the (obscene) level of complexity that MS has put in the MSVC initialization logic.. |
|
@jcbrill - is there anyway to have vcvarsall.bat (or equivalent) skip vcpkg initialization? |
No such luck. I'm leaning towards the slowdown was somehow due to running the powershell script with powershell 5 and without the It appears that adding powershell 7 in front of powershell 5 in the PATH and setting the I can try in my copy of the windows runner environment using powershell 5 and setting the The vcpkg batch files are written to use whichever version of powershell it finds first on the path. The vcpkg.bat ...
call "%VSINSTALLDIR%VC\vcpkg\vcpkg-init.cmd" >nul 2>&1
set PATH=%PATH%;%VSINSTALLDIR%VC\vcpkg
...vcpkg-init.cmd @(echo off) > $null
if #ftw NEQ '' goto :init
($true){ $Error.clear(); }
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT License.
# wrapper script for vcpkg
# this is intended to be dot-sourced and then you can use the vcpkg-shell() function
# Workaround for $IsWindows not existing in Windows PowerShell
if (-Not (Test-Path variable:IsWindows)) {
$IsWindows = $true
}
function download($url, $path) {
$wc = New-Object net.webclient
Write-Host "Downloading '$url' -> '$path'"
$wc.DownloadFile($url, $path);
$wc.Dispose();
if( (get-item $path).Length -ne $wc.ResponseHeaders['Content-Length'] ) {
throw "Download of '$url' failed. Check your internet connection."
}
if (-Not $IsWindows) {
chmod +x $path
}
return $path
}
# Determine VCPKG_ROOT
if (Test-Path "$PSScriptRoot/.vcpkg-root") {
$env:VCPKG_ROOT = "$PSScriptRoot"
} elseif (-Not (Test-Path env:VCPKG_ROOT)) {
$env:VCPKG_ROOT = "$HOME/.vcpkg"
}
$VCPKG = Join-Path $env:VCPKG_ROOT vcpkg
if ($IsWindows) {
$VCPKG += '.exe'
}
$SCRIPT:VCPKG_VERSION_MARKER = Join-Path $env:VCPKG_ROOT vcpkg-version.txt
$SCRIPT:VCPKG_INIT_VERSION = '2024-12-09'
function bootstrap-vcpkg {
if (-Not ($VCPKG_INIT_VERSION -eq 'latest') `
-And (Test-Path $VCPKG_VERSION_MARKER) `
-And ((Get-Content -Path $VCPKG_VERSION_MARKER -Raw).Trim() -eq $VCPKG_INIT_VERSION)) {
return $True
}
Write-Host "Installing vcpkg to $env:VCPKG_ROOT"
New-Item -ItemType Directory -Force $env:VCPKG_ROOT | Out-Null
if ($IsWindows) {
download https://github.com/microsoft/vcpkg-tool/releases/download/2024-12-09/vcpkg.exe $VCPKG
} elseif ($IsMacOS) {
download https://github.com/microsoft/vcpkg-tool/releases/download/2024-12-09/vcpkg-macos $VCPKG
} elseif (Test-Path '/etc/alpine-release') {
download https://github.com/microsoft/vcpkg-tool/releases/download/2024-12-09/vcpkg-muslc $VCPKG
} else {
download https://github.com/microsoft/vcpkg-tool/releases/download/2024-12-09/vcpkg-glibc $VCPKG
}
& $VCPKG bootstrap-standalone
if(-Not $?) {
Write-Error "Bootstrap failed."
return $False
}
Write-Host "Bootstrapped vcpkg: $env:VCPKG_ROOT"
return $True
}
if(-Not (bootstrap-vcpkg)) {
throw "Unable to install vcpkg."
}
# Export vcpkg to the current shell.
New-Module -name vcpkg -ArgumentList @($VCPKG) -ScriptBlock {
param($VCPKG)
function vcpkg-shell() {
# setup the postscript file
# Generate 31 bits of randomness, to avoid clashing with concurrent executions.
$env:Z_VCPKG_POSTSCRIPT = Join-Path ([System.IO.Path]::GetTempPath()) "VCPKG_tmp_$(Get-Random -SetSeed $PID).ps1"
& $VCPKG @args
# dot-source the postscript file to modify the environment
if (Test-Path $env:Z_VCPKG_POSTSCRIPT) {
$postscr = Get-Content -Raw $env:Z_VCPKG_POSTSCRIPT
if( $postscr ) {
iex $postscr
}
Remove-Item -Force -ea 0 $env:Z_VCPKG_POSTSCRIPT
}
Remove-Item env:Z_VCPKG_POSTSCRIPT
}
} | Out-Null
if ($args.Count -ne 0) {
return vcpkg-shell @args
}
return
<#
:init
:: If the first line of this script created a file named $null, delete it
IF EXIST $null DEL $null
:: Figure out where VCPKG_ROOT is
IF EXIST "%~dp0.vcpkg-root" (
SET "VCPKG_ROOT=%~dp0"
)
IF "%VCPKG_ROOT:~-1%"=="\" (
SET "VCPKG_ROOT=%VCPKG_ROOT:~0,-1%"
)
IF "%VCPKG_ROOT%"=="" (
SET "VCPKG_ROOT=%USERPROFILE%\.vcpkg"
)
:: Call powershell which may or may not invoke bootstrap if there's a version mismatch
SET Z_POWERSHELL_EXE=
FOR %%i IN (pwsh.exe powershell.exe) DO (
IF EXIST "%%~$PATH:i" (
SET "Z_POWERSHELL_EXE=%%~$PATH:i"
GOTO :gotpwsh
)
)
:gotpwsh
"%Z_POWERSHELL_EXE%" -NoProfile -ExecutionPolicy Unrestricted -Command "iex (get-content \"%~dfp0\" -raw)#"
IF ERRORLEVEL 1 (
:: leak VCPKG_ROOT
SET Z_POWERSHELL_EXE=
EXIT /B 1
)
SET Z_POWERSHELL_EXE=
:: Install the doskey
DOSKEY vcpkg-shell="%VCPKG_ROOT%\vcpkg-cmd.cmd" $*
:: If there were any arguments, also invoke vcpkg with them
IF "%1"=="" GOTO fin
CALL "%VCPKG_ROOT%\vcpkg-cmd.cmd" %*
:fin
EXIT /B
#> |
|
So this kind of illustrates my "fragile" quip from somewhere else in this general thread, which offended @bdbaddog without my meaning to. Here a working non-SCons environment includes a cascade of scripts that includes picking up ones that have been dropped into the searched directories sort of plugin-like by a non-default install ( Of course the option still remains for the project to add those themselves to Sorry, not sure if this added to the PR review. |
|
Given the above, sounds like we could also remove the bat script in that dir which setup vcpkg on the GHA VM with similar removal of slowdown? |
The msvc environment is built from a new environment and then only extracts certain variables. See LeonarddeR/scons-examples@fa9402a#commitcomment-156340060 and the preceding post. |
|
Observation: the elapsed times can be wildly variable. Guessing due to possible networking and server latency. Three combinations of runs:
Having the powershell 7 modules on the PSModulePath appears to make a big difference (2 & 3). I'm guessing that with a proper experiment configuration, the elapsed times are similar using powershell 7 (3) versus (5). This PR configures the paths for 3 above. It doesn't make a lot of sense to have the powershell 7 modules first and use powershell 5. Seems like it asking for trouble. @bdbaddog That is why adding the PSModulePath to the environment worked well in our early tests. The test results above did not set I can try the tests again early tomorrow morning as the elapsed times tend to be lower. |
Yes. Although one could enable every option in the VS installer and manually check and see what ends up in the ext script and then follow any additional calls. Manual labor intensive and error prone. By setting To your point, it looks like Some of these features may be important if one is building from ms vsproject files and far less important for SCons builds. I'm am sure to be wrong here though.
The msvc batch file environment is not passed the current Environment but creates "a blank environment, for use in launching the tools".
This is all important moving forward. |
I'm not sure modifying the installation without knowing exactly how it is being used is advisable. It would seem possible that MSVC could be used in other ways in addition to running SCons in the same job. |
In the context of running SCons tests on GHA windows VM, it would be safe. |
|
There really is not that much to this PR. We are adding the following only for the msvc batch file invocation:
@bdbaddog Is it possible to setup a windows runner (2022) environment to download an scons branch and run scons manually? For example, this how I run the development branch with SCons locally from a test folder: Basically, can we test the proposed branch in a windows runner environment prior to accepting the PR to work out the kinks (if any)? |
|
I have reason to believe that simply adding powershell 7 to the msvc environment path in front of powershell 5 is the solution and that adding PSModulePath is not necessary. I believe when a powershell binary is invoked, if Notes:
Sample output from windows runner 2022 test environment (PATH with PS7, No PSModulePATH): |
|
You removed |
I did remove PSModulePath. There is a subtle difference between what was set in the For the earlier version, two module locations for each version of powershell were added: the "All Users" powershell location and the powershell installation location. I believe that powershell may include a current user location first before the other locations when Argument for taking it out: If it works without The only case that probably doesn't work is if somehow powershell is installed to a different file system location but the modules were in the expected locations. In this case, powershell 5 would need the The vcpkg batch files are simply searching the path for the executable and running invoking it relying on the vcpkg executable to do the heavy lifting for finding modules. I'm leaning towards not defining Any thoughts on how to test the branch in a windows runner 2022 environment? Disclaimer: I know diddly squat about powershell. |
|
Share the lack of knowledge about PS. I think if you make a change to the workflow like this patch (untested, because how do you test it locally?), it should try a build on 2022. diff --git a/.github/workflows/runtest-win.yml b/.github/workflows/runtest-win.yml
index 2dc2ce118..11d17e48f 100644
--- a/.github/workflows/runtest-win.yml
+++ b/.github/workflows/runtest-win.yml
@@ -18,7 +18,15 @@ env:
jobs:
runtest-win32:
- runs-on: windows-latest
+
+ strategy:
+ fail-fast: false
+ matrix:
+ os: ['windows-2022', 'windows-2025']
+
+ # runs-on: windows-latest
+ runs-on: ${{ matrix.os }}
+
steps:
- uses: actions/[email protected]
There seems little use bothering with a test on 2019, due to this note:
|
|
Thanks. I'm not really looking to run the full test suite with the branch but to run one simple build with the branch (like one of the SCons examples) to check the build time. I don't think a new test will work due to the msvc cache. A separate workflow/action? Any thoughts? |
|
Sure - we can run whatever, there's a reasonable bit of control in setting up an action. I'm no expert on those things either, just muddled through a few times. |
|
What I'm thinking is something stupid simple like this:
That should call vcvars64.bat once? And then the magic happens here... |
Changes: * change raw string with multiple directories comma-separated list of string directories for powershell 7 * move constant list of environment variable names definition from inside function to file level and rename accordingly.
SCons/Tool/MSCommon/common.py
Outdated
| # control execution in interesting ways. | ||
| # Note these really should be unified - either controlled by vs.py, | ||
| # or synced with the the common_tools_var # settings in vs.py. | ||
| _VS_VC_VARS = [ |
There was a problem hiding this comment.
I think you can remove the _ prefix here. There's no major reason to sugguest accessing this from other code should be avoided ?
There was a problem hiding this comment.
It is an indicator that it shouldn't necessarily be modified. I don't feel strongly either way.
We should probably add a debug logging statement for each of the keys retrieved from the list in normalize_env since the list can now be modified. The mscommon log file is the easiest way to determine if a user modified the list.
There was a problem hiding this comment.
Pushed change removing the _ prefix and added debug logging in the loop that evaluates the keys.
…ronment variable list processing.
…nt in which the MSVC batch files run. Update CHANGES.txt and RELEASE.txt.
jcbrill
changed the title
jcbrill
changed the title
|
At present, no additional work planned unless requested. |
…y this PR. Also moved list of shell env vars propaagated to msvs configuration environment to top of file as is fairly standard practice
|
@jcbrill - just pushed a couple (IMHO) minor updates, I wanted to better highlight what's fixed by this PR and then just moved the list of shell variables initialized into the constructed environment to run init script to top of file as we'd discussed above. Assuming tests pass, I'll go ahead and merge this. |
Evidently without looking at the rest of the file or the other file-level variable list and two constants would have been moved as well. One list just moved from the only place it is used to the top without a particularly useful name. IMHO, this was not an improvement. |
|
The runtime improvement is due to using powershell 7 instead of powershell 5 in the windows runner. If powershell 7 is not installed in a similar environment, the runtime penalty may still exist. |
I'll move the rest then as well.. |
Might as well make all the code harder to read. |
|
The CHANGES.txt text changed from a precise statement about what was changed and the environment in which the change would apply, to something that may not hold in all cases. - - The default Windows powershell 7 path is added before the default
- Windows powershell 5 path in the limited environment in which the
- MSVC batch files are run. This appears to fix a runtime issue in
- the windows runner environment when the embedded Visual Studio vcpkg
- component initializes without a PSModulePath defined. The OS
+ - MSVS: Fix a significant MSVC/MSVC tool initialization slowdown when
+ vcpkg has been installed and the PSModulePath is not initialized
+ or propagated from the user's shell environment. To resolve this
+ The default Windows Powershell 7 path is added before the default
+ Windows Powershell 5 path in the carefully constructed
+ environment in which the MSVC batch files are run. The shellThe runtime improvement seen in the windows runner environment was due to PS 7 and VS vcpkg being installed at the same time. Powershell 7 is an optional component install in Windows (i.e., not installed by default). As mentioned above, in the windows runner environment:
Adding the PS 7 path in an environment in which PS 7 is not installed is unlikely to result in any improvement and there may be possible runtime degradation if the environment is similar to the windows runner environment. In the future, if the text I've written for work that I performed and is attributed to me is modified, I would appreciate it if:
There was a reason the original text was written the way it was. |
|
Please PR an update, but pay attention to the below. The issue I saw with your text is it was implementation focused (what changed in the implementation) vs what it fixed for a user having an issue. That should always be the focus of the CHANGES.txt and RELEASE.txt. I updated to be more user focused as I understood the issue, which according to the above was incorrect. |
|
Fair request to review, but I was just short on time and thought the change was correct. |
The intention was well-meaning. More could, possibly unintentionally, be read into the modified language that was not necessarily present in the original. The concern is that the improvement is seen in one specific combination of OS and installed tools and may not translate to other runtime environments. It is fair to say there was a significant improvement in the windows runner 2022 environment. Unfortunately, that is the only environment known to be experiencing a runtime issue which benefits significantly from the fix in the PR. The runtime issue couldn't be replicated locally with Windows 11 and VS 2022 Enterprise both with and without PS 7 installed. PS 5 seemed to work well enough. The significant slowdowns were not seen locally and the relative elapsed times seemed reasonably close enough. If one is using the windows runner (2022), there would certainly be a good reason to upgrade to the latest version. For other combinations, there may not be any benefit, or even worse, the initialization delays might be unavoidable. Although, adopting the latest version shouldn't make it worse (I hope). While a mostly likely cause was identified, there is still uncertainty as to the exact step(s) in the vcpkg initialization sequence that experience the delays in the powershell script(s). Of concern is what the vcpkg initialization runtime in a windows-runner-like environment would be if PS 7 was not installed. Would users be "stuck" with the initialization slowdown without remedy? There may also be OS/component combinations in the wild that experience the same initialization slowdown that goes largely unnoticed because it is a minor fraction of the full build time. For example, the SCons test suite running with the MSVC cache. A 60 second slowdown may be experienced for the first invocation of the msvc batch files. However, the magnitude of the slowdown is almost meaningless when the full test suite is run with the cache. In this example, a significant improvement in vcpkg initialization time has an inconsequential impact on overall runtime. Without the cache would be a different story. The cost of the slowdown would be additive across all tests that have default tools initialization and/or use of the default compilers. |
|
Notes for updated text PR. Observations:
The runtimes with powershell 5 are an order of magnitude slower than with powershell 7 in Visual Studio 2022 VCPKG
Example SConstruct times with Powershell 5 (runner results sorted low to high in secs):
Example SConstruct times with Powershell 7 (runner results sorted low to high in secs):
|
|
@jcbrill better to just start a PR and include the above? |
Changes: * Partially restore restore original GH SCons#4717 CHANGES.txt description * Change MSVS to MSVC in RELEASE.txt * Remove extraneous two blank lines introduced in common.py when variable moved
Fix for vcpkg run time in windows runner (VS2022 Enterprise).
Changes to msvc batch file invocation environment:
Contributor Checklist:
CHANGES.txtandRELEASE.txt(and read theREADME.rst).