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

Remove Zig's internal depedency on shell32.dll and ole32.dll #18091

Merged
merged 4 commits into from
Nov 23, 2023

Conversation

squeek502
Copy link
Collaborator

@squeek502 squeek502 commented Nov 23, 2023

This is mostly a proof-of-concept. Contributes towards #1840 although that's not the primary motivation.

Motivation

viri on Discord posted this article when we were talking about command line parsing and reasons for avoiding CommandLineToArgvW:

https://randomascii.wordpress.com/2018/12/03/a-not-called-function-can-cause-a-5x-slowdown/

The post details a considerable slowdown on process initialization whenever a process depends on shell32.dll (or ole32.dll by extension since it depends on shell32.dll), even if none of the shell32/ole32 functions are called during the runtime of the program. This is due to shell32.dll bringing in gdi32.dll and causing some GDI-related initialization at process startup.

This PR

This PR removes all dependencies on shell32/ole32 from Zig code, and removes std.os.windows.shell32/std.os.windows.ole32 entirely from the standard library.

shell32.SHGetKnownFolderPath

This function was used to get the local app data directory in std.fs.getAppDataDir. This PR removes the SHGetKnownFolderPath usage and instead just uses the environment variable LOCALAPPDATA. This is not a perfect solution, as SHGetKnownFolderPath does some complicated acrobatics with registry/user profile stuff.

If this PR is merged, then a follow-up issue will be created for a proper SHGetKnownFolderPath reimplementation (or maybe a proper SHGetKnownFolderPath implementation could be a blocker for this PR). This would likely be based on Wine's.

ole32 COM-related stuff

These functions were used in windows_sdk.zig to get Visual Studio installation instances via ISetupConfiguration. As noted in #16594 (comment) and #16594 (comment), the COM stuff under-the-hood was actually just reading .json files from a Microsoft\VisualStudio\Packages\_Instances directory in %PROGRAMDATA%.

So, windows_sdk.zig now just reads those .json files directly instead of going through the COM interface. If you're interested, here's a full log from Procmon of what all would happen during the previous COM-based implementation:

https://gist.github.com/squeek502/2aa0b9498c5552fa887b1556291f8745

If you'd like to test the zig libc of this PR to make sure it still works on your setup, you can run this build of libc_only.exe:

libc_only-20231123.zip

The encouraging results

No shell32.dll/ole32.dll in a build of the compiler without LLVM:

> dumpbin /dependents /nologo .\stage4-new\bin\zig.exe

  Image has the following dependencies:

    ntdll.dll
    KERNEL32.dll
    WS2_32.dll
    ADVAPI32.dll
    CRYPT32.dll

Compilers built without LLVM, printing the --help text:

Benchmark 1: .\stage4-master\bin\zig.exe --help
  Time (mean ± σ):       6.7 ms ±   0.8 ms    [User: 4.6 ms, System: 6.2 ms]
  Range (min … max):     5.7 ms …  10.9 ms    97 runs

Benchmark 2: .\stage4-new\bin\zig.exe --help
  Time (mean ± σ):       4.1 ms ±   0.7 ms    [User: 4.1 ms, System: 5.6 ms]
  Range (min … max):     3.0 ms …   6.4 ms    122 runs

Summary
  '.\stage4-new\bin\zig.exe --help' ran
    1.62 ± 0.33 times faster than '.\stage4-master\bin\zig.exe --help'

libc_only.exe as detailed here:

Benchmark 1: libc_only-master.exe
  Time (mean ± σ):      10.7 ms ±   0.8 ms    [User: 5.8 ms, System: 7.8 ms]
  Range (min … max):     9.1 ms …  13.8 ms    91 runs

Benchmark 2: libc_only.exe
  Time (mean ± σ):       6.3 ms ±   0.5 ms    [User: 5.1 ms, System: 5.5 ms]
  Range (min … max):     5.4 ms …   8.3 ms    113 runs

Summary
  'libc_only.exe' ran
    1.70 ± 0.20 times faster than 'libc_only-master.exe'

The dashed hopes

Unfortunately, LLVMSupport.lib has its own dependencies on shell32's SHGetKnownFolderPath/SHFileOperationW and ole32's CoTaskMemFree:

which means that when building Zig with LLVM enabled, we inherit those dependencies and the slow process initialization returns, even if the shell32/ole32 functions aren't called. So, there's no actual gain in process initialization at all in status-quo Zig.

The meager reality

The new windows_sdk.zig is faster than the previous implementation, but not too much (these are ReleaseFast compilers built with LLVM enabled):

> hyperfine ".\stage4-master\bin\zig.exe libc" ".\stage4-new\bin\zig.exe libc"
Benchmark 1: .\stage4-master\bin\zig.exe libc
  Time (mean ± σ):      16.5 ms ±   0.8 ms    [User: 9.6 ms, System: 10.3 ms]
  Range (min … max):    15.0 ms …  19.0 ms    80 runs

Benchmark 2: .\stage4-new\bin\zig.exe libc
  Time (mean ± σ):      14.3 ms ±   0.8 ms    [User: 8.1 ms, System: 9.6 ms]
  Range (min … max):    13.1 ms …  17.6 ms    89 runs

Summary
  '.\stage4-new\bin\zig.exe libc' ran
    1.16 ± 0.09 times faster than '.\stage4-master\bin\zig.exe libc'

The path forward

Eventually, #16270 will come into play and Zig's lack of dependency on shell32.dll/ole32.dll will become meaningful, so investment in shedding these dependencies should be worthwhile in the long run.

In the meantime, there is a potential avenue to explore with regards to -delayload which is what LLVM uses to avoid the process initialization cost of the shell32/ole32 dependency. I haven't put much effort into investigating this or if/how it could be utilized by Zig, but it might be possible. My naive attempt at building LLVM from source with MSVC and then building Zig with that didn't seem to get the -delayload to carry over, but again I didn't look too closely/try much of anything.

Removes the dependency on shell32.dll. This is a stop gap solution until a proper SHGetKnownFolderPath Zig implementation is written
@andrewrk
Copy link
Member

Excellent work! I love the dramatic, insightful writeup.

@andrewrk andrewrk merged commit 464ce8a into ziglang:master Nov 23, 2023
10 checks passed
@daurnimator
Copy link
Contributor

@squeek502 it might be worth adding a little CI check that zig doesn't depend on shell32.dll again? Otherwise this is likely to accidentally regress with some refactor

@squeek502
Copy link
Collaborator Author

squeek502 commented Nov 24, 2023

it might be worth adding a little CI check that zig doesn't depend on shell32.dll again? Otherwise this is likely to accidentally regress with some refactor

Note that the compiler still depends on shell32 when built with LLVM enabled, so I think this would be better to do once the version of the compiler that actually gets shipped stops depending on shell32. Within .zig code, it should be fairly obvious if this was going to regress and it's unlikely to do so because of to the already-existing scrutiny on dll dependencies via #1840

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 this pull request may close these issues.

3 participants