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

Use the Universal C Runtime #888

Merged
merged 2 commits into from
Jun 3, 2021
Merged

Use the Universal C Runtime #888

merged 2 commits into from
Jun 3, 2021

Conversation

DrusTheAxe
Copy link
Member

@DrusTheAxe DrusTheAxe commented May 30, 2021

Update Microsoft.ProjectReunion.dll, Microsoft.ProjectReunion.Bootstrap.dll, DynamicDependency DataStore and LifetimeManager to use the Universal CRT (like MRTCore's been doing) and VS' static CRT. This removes the dependency on the VS CRT redistributable at negible cost (see below).

Static analysis before/after using LINK /DUMP /IMPORTS, linker options /MAP /VERBOSE and SizeBench confirms the trivial size delta.

Disk size of x64 Binaries

File Size Before Size After Delta
DynamicDependency.DataStore.exe 58,368 81,408 23,040
DynamicDependency.DataStore.ProxyStub.dll 13,312 24,064 10,752
DynamicDependencyLifetimeManager.exe 41,984 64,000 22,016
DynamicDependencyLifetimeManager.ProxyStub.dll 13,312 24,064 10,752
Microsoft.ProjectReunion.dll 516,096 554,496 38,400
Microsoft.ProjectReunion.Bootstrap.dll 80,896 104,448 23,552
Total 723,968 852,480 128,512

SizeBench delta for Microsoft.ProjectReunion.dll

Name Size on disk Diff Size in memory Diff Size in memory (including section padding) Diff
.text 23552 23637 24576
.rdata 11264 11432 12288
.pdata 1536 1644 0
.data 1024 2816 4096
.reloc 512 532 0
_RDATA 512 244 4096
.detourc 0 0 0
.detourd 0 0 0
.rsrc 0 0 0

SizeBench delta for Microsoft.ProjectReunion.Bootstrap.dll

Name Size on disk Diff Size in memory Diff Size in memory (including section padding) Diff
.text 15872 16176 16384
.rdata 5120 5272 4096
.pdata 1024 996 4096
.data 512 480 0
.reloc 512 256 0
_RDATA 512 252 4096
.didat 0 0 0
.rsrc 0 0 0

The .vcxproj changes are based the implementation in MRM.vcxproj and BaseUnitTests.vcxproj

  1. Add UseUniCrt=true to the 'Globals' PropertyGroup
  2. Added RuntimeLibrary=MultiThreadedDebug (Debug) or MultiThreaded (Release)
  3. Add IgnoreSpecificDefaultLibraries=libucrtd.lib (Debug) or libucrt.lib (Release)
  4. Add AdditionalOptions /defaultlib:ucrtd.lib (Debug) or /defaultlib:ucrt.lib (Release)

Snippets below:

#1 UseUnicrt

  <PropertyGroup Label="Globals">
    <UseUnicrt>true</UseUnicrt>

#2-4 ClCompile and Link options

  <ItemDefinitionGroup Condition="'$(Configuration)'=='Debug'">
      <!-- We use MultiThreadedDebug, rather than MultiThreadedDebugDLL, to avoid DLL dependencies on VCRUNTIME140d.dll and MSVCP140d.dll. -->
      <RuntimeLibrary>MultiThreadedDebug</RuntimeLibrary>
    </ClCompile>
    <Link>
      <!-- Link statically against the runtime and STL, but link dynamically against the CRT by ignoring the static CRT
           lib and instead linking against the Universal CRT DLL import library. This "hybrid" linking mechanism is
           supported according to the CRT maintainer. Dynamic linking against the CRT makes the binaries a bit smaller
           than they would otherwise be if the CRT, runtime, and STL were all statically linked in. -->
      <IgnoreSpecificDefaultLibraries>%(IgnoreSpecificDefaultLibraries);libucrtd.lib</IgnoreSpecificDefaultLibraries>
      <AdditionalOptions>%(AdditionalOptions) /defaultlib:ucrtd.lib</AdditionalOptions>
    </Link>
  </ItemDefinitionGroup>
  <ItemDefinitionGroup Condition="'$(Configuration)'=='Release'">
    <ClCompile>
      <!-- We use MultiThreaded, rather than MultiThreadedDLL, to avoid DLL dependencies on VCRUNTIME140.dll and MSVCP140.dll. -->
      <RuntimeLibrary>MultiThreaded</RuntimeLibrary>
    </ClCompile>
    <Link>
      <!-- Link statically against the runtime and STL, but link dynamically against the CRT by ignoring the static CRT
           lib and instead linking against the Universal CRT DLL import library. This "hybrid" linking mechanism is
           supported according to the CRT maintainer. Dynamic linking against the CRT makes the binaries a bit smaller
           than they would otherwise be if the CRT, runtime, and STL were all statically linked in. -->
      <IgnoreSpecificDefaultLibraries>%(IgnoreSpecificDefaultLibraries);libucrt.lib</IgnoreSpecificDefaultLibraries>
      <AdditionalOptions>%(AdditionalOptions) /defaultlib:ucrt.lib</AdditionalOptions>
    </Link>
  </ItemDefinitionGroup>

This really should go into a common UniversalCrt.props like other common properties (no reason all projects shouldn't use this) along with stripping unwanted options and/or verifying no inconsistencies but those build infrastructure refinements are left as an exercise for a future PR. First step is to use UCRT for product.

@DrusTheAxe DrusTheAxe added this to the 1.0 (2021 Q4) milestone May 30, 2021
@DrusTheAxe DrusTheAxe self-assigned this May 30, 2021
@ghost ghost added the needs-triage label May 30, 2021
@DrusTheAxe DrusTheAxe added area-Infrastructure Build, test, source layout, package construction (TODO: move to Deployment, DeveloperTools) area-DynamicDependencies area-Lifecycle and removed needs-triage labels May 30, 2021
@DrusTheAxe DrusTheAxe requested a review from kythant May 30, 2021 22:30
@riverar
Copy link
Contributor

riverar commented May 31, 2021

To other reviewers: If you're like me, you keep falling for this trick. There's no security issue here, see #573 (comment) for more context. 👍

@DrusTheAxe DrusTheAxe linked an issue May 31, 2021 that may be closed by this pull request
@DrusTheAxe
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@DrusTheAxe DrusTheAxe marked this pull request as ready for review May 31, 2021 03:11
@DrusTheAxe DrusTheAxe enabled auto-merge (squash) May 31, 2021 03:11
@DrusTheAxe DrusTheAxe requested a review from astritz June 1, 2021 19:36
@Scottj1s
Copy link
Member

Scottj1s commented Jun 3, 2021

I am in favor of this strategy, and love the analysis above which everyone has been asking for. As you say, the tax is negligible and a fair trade-off for eliminating CRT redist woes for the end developer. That said, the competition for this is the VCLibs.UWPDesktop converged framework package and dynamic dependency deployment (yes?). Should pros/cons of that option be considered and included?

@DrusTheAxe
Copy link
Member Author

VCLibs.UWPDesktop converged framework package and dynamic dependency deployment (yes?). Should pros/cons of that option be considered and included?

No. They're not really equivalent if you consider process startup and loader dependencies.

VCLibs framework package is cool for packaged processes as they're born with the manifested dependency in their loader search order. Not so for unpackaged apps. For non-C/C++ modules it's no big deal since they have no imports to the CRT so e.g. C# foo.exe can run, use DynDep to load VCLibs.UWPDesktop than use CRT APIs. For C/C++ (and Rust? Others?) modules they'd with any IAT entries for the CRT would have to delayload the CRT. Possible but irksome so the added friction is unappealing.

The small tax for this hybrid approach is a more compelling answer, at least for Project Reunion.

@DrusTheAxe DrusTheAxe closed this Jun 3, 2021
auto-merge was automatically disabled June 3, 2021 18:40

Pull request was closed

@DrusTheAxe DrusTheAxe reopened this Jun 3, 2021
@ghost ghost added the needs-triage label Jun 3, 2021
@DrusTheAxe DrusTheAxe enabled auto-merge (squash) June 3, 2021 18:44
@DrusTheAxe
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@DrusTheAxe DrusTheAxe merged commit 2af7b0f into main Jun 3, 2021
@DrusTheAxe DrusTheAxe deleted the user/drustheaxe/crt branch June 3, 2021 19:14
DrusTheAxe added a commit that referenced this pull request Jun 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-DynamicDependencies area-Infrastructure Build, test, source layout, package construction (TODO: move to Deployment, DeveloperTools) area-Lifecycle needs-triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update ProjectReunion to use UCRT
4 participants