Move src/native libraries that are shared between runtime builds to be targets-driven by jkoritzinsky · Pull Request #103184 · dotnet/runtime

jkoritzinsky · 2024-06-08T00:25:25Z

Today, the src/native/containers and src/native/eventpipe libraries are defined in terms of source and header file lists.

This provides flexibility into how they're defined in CMake, but it makes it difficult to flow include directories or dependencies. It also makes it difficult to encapsulate logic, as these lists are defined in .cmake files that are included into a CMakeLists.txt file.

This PR converts these libraries to be defined in terms of CMake targets.

src/native/containers is transformed into an object library, so we can build it once and then link it into static libs, shared libs, or executables. An extern "C" block was added around all declarations that are currently used in eventpipe/diagnosticserver (so not dn-simdhash) to enable the build to build the containers library as a C library. Also, a default implementation of the dn_simdhash_assert_fail method was added to enable the containers lib to link into runtimes that don't currently provide an implementation.

src/native/eventpipe is transformed into interface libraries. Due to how eventpipe and diagnosticserver both require runtime-specific headers and need to be built with different flags for each target, defining them as interface libraries is the cleanest manner to handle this scenario (until we can refactor the ep-rt-*.h and ds-rt-*.h headers into a cleaner design).

This PR also updates the eventpipe and diagnosticserver codebases to use CMake's UNITY_BUILD support in each runtime's implementation instead of manually implementing a unity build.

dotnet-policy-service · 2024-06-08T00:26:00Z

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

lambdageek · 2024-06-10T18:51:00Z

FYI there's some eventpipe and containers unit tests in https://github.com/dotnet/runtime/tree/main/src/mono/mono/eventpipe/test that (AFAIK) don't run on any pipeline, but that is used for local validation of EP or container changes

…sources

… for runtimes that haven't adopted simdhash

…raries, and update CoreCLR to use the new model.

…rsion.

… there's very few reasons to use it in this way, but this is one of the scenarios for it).

…sources.

…ild.

…will fix some of the NativeAOT warnings

lambdageek · 2024-06-24T14:42:02Z

+#include <stdio.h>
+#include <stdlib.h>
+
+// Define a default implementation of the assert failure function.


Should we put something like this in minipal? it seems generally useful

I think I'd like to see more places where we'd use it before I move it to minipal. We can extract it the next time we'd have a use for it.

jkoritzinsky · 2024-06-24T20:42:20Z

/ba-g Wasm firefox timeout seems unrelated.

jkoritzinsky · 2024-06-08T00:25:25Z

Today, the src/native/containers and src/native/eventpipe libraries are defined in terms of source and header file lists.

This provides flexibility into how they're defined in CMake, but it makes it difficult to flow include directories or dependencies. It also makes it difficult to encapsulate logic, as these lists are defined in .cmake files that are included into a CMakeLists.txt file.

This PR converts these libraries to be defined in terms of CMake targets.

src/native/containers is transformed into an object library, so we can build it once and then link it into static libs, shared libs, or executables. An extern "C" block was added around all declarations that are currently used in eventpipe/diagnosticserver (so not dn-simdhash) to enable the build to build the containers library as a C library. Also, a default implementation of the dn_simdhash_assert_fail method was added to enable the containers lib to link into runtimes that don't currently provide an implementation.

src/native/eventpipe is transformed into interface libraries. Due to how eventpipe and diagnosticserver both require runtime-specific headers and need to be built with different flags for each target, defining them as interface libraries is the cleanest manner to handle this scenario (until we can refactor the ep-rt-*.h and ds-rt-*.h headers into a cleaner design).

This PR also updates the eventpipe and diagnosticserver codebases to use CMake's UNITY_BUILD support in each runtime's implementation instead of manually implementing a unity build.

dotnet-policy-service · 2024-06-08T00:26:00Z

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

lambdageek · 2024-06-10T18:51:00Z

FYI there's some eventpipe and containers unit tests in https://github.com/dotnet/runtime/tree/main/src/mono/mono/eventpipe/test that (AFAIK) don't run on any pipeline, but that is used for local validation of EP or container changes

…sources

… for runtimes that haven't adopted simdhash

…raries, and update CoreCLR to use the new model.

…rsion.

… there's very few reasons to use it in this way, but this is one of the scenarios for it).

…sources.

…ild.

…will fix some of the NativeAOT warnings

lambdageek · 2024-06-24T14:42:02Z

+#include <stdio.h>
+#include <stdlib.h>
+
+// Define a default implementation of the assert failure function.


Should we put something like this in minipal? it seems generally useful

I think I'd like to see more places where we'd use it before I move it to minipal. We can extract it the next time we'd have a use for it.

jkoritzinsky · 2024-06-24T20:42:20Z

/ba-g Wasm firefox timeout seems unrelated.

jkoritzinsky · 2024-06-08T00:25:25Z

Today, the src/native/containers and src/native/eventpipe libraries are defined in terms of source and header file lists.

This provides flexibility into how they're defined in CMake, but it makes it difficult to flow include directories or dependencies. It also makes it difficult to encapsulate logic, as these lists are defined in .cmake files that are included into a CMakeLists.txt file.

This PR converts these libraries to be defined in terms of CMake targets.

src/native/containers is transformed into an object library, so we can build it once and then link it into static libs, shared libs, or executables. An extern "C" block was added around all declarations that are currently used in eventpipe/diagnosticserver (so not dn-simdhash) to enable the build to build the containers library as a C library. Also, a default implementation of the dn_simdhash_assert_fail method was added to enable the containers lib to link into runtimes that don't currently provide an implementation.

src/native/eventpipe is transformed into interface libraries. Due to how eventpipe and diagnosticserver both require runtime-specific headers and need to be built with different flags for each target, defining them as interface libraries is the cleanest manner to handle this scenario (until we can refactor the ep-rt-*.h and ds-rt-*.h headers into a cleaner design).

This PR also updates the eventpipe and diagnosticserver codebases to use CMake's UNITY_BUILD support in each runtime's implementation instead of manually implementing a unity build.

dotnet-policy-service · 2024-06-08T00:26:00Z

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

lambdageek · 2024-06-10T18:51:00Z

FYI there's some eventpipe and containers unit tests in https://github.com/dotnet/runtime/tree/main/src/mono/mono/eventpipe/test that (AFAIK) don't run on any pipeline, but that is used for local validation of EP or container changes

…sources

… for runtimes that haven't adopted simdhash

…raries, and update CoreCLR to use the new model.

…rsion.

… there's very few reasons to use it in this way, but this is one of the scenarios for it).

…sources.

…ild.

…will fix some of the NativeAOT warnings

lambdageek · 2024-06-24T14:42:02Z

+#include <stdio.h>
+#include <stdlib.h>
+
+// Define a default implementation of the assert failure function.


Should we put something like this in minipal? it seems generally useful

I think I'd like to see more places where we'd use it before I move it to minipal. We can extract it the next time we'd have a use for it.

jkoritzinsky · 2024-06-24T20:42:20Z

/ba-g Wasm firefox timeout seems unrelated.

jkoritzinsky · 2024-06-08T00:25:25Z

Today, the src/native/containers and src/native/eventpipe libraries are defined in terms of source and header file lists.

This provides flexibility into how they're defined in CMake, but it makes it difficult to flow include directories or dependencies. It also makes it difficult to encapsulate logic, as these lists are defined in .cmake files that are included into a CMakeLists.txt file.

This PR converts these libraries to be defined in terms of CMake targets.

src/native/containers is transformed into an object library, so we can build it once and then link it into static libs, shared libs, or executables. An extern "C" block was added around all declarations that are currently used in eventpipe/diagnosticserver (so not dn-simdhash) to enable the build to build the containers library as a C library. Also, a default implementation of the dn_simdhash_assert_fail method was added to enable the containers lib to link into runtimes that don't currently provide an implementation.

src/native/eventpipe is transformed into interface libraries. Due to how eventpipe and diagnosticserver both require runtime-specific headers and need to be built with different flags for each target, defining them as interface libraries is the cleanest manner to handle this scenario (until we can refactor the ep-rt-*.h and ds-rt-*.h headers into a cleaner design).

This PR also updates the eventpipe and diagnosticserver codebases to use CMake's UNITY_BUILD support in each runtime's implementation instead of manually implementing a unity build.

dotnet-policy-service · 2024-06-08T00:26:00Z

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

lambdageek · 2024-06-10T18:51:00Z

FYI there's some eventpipe and containers unit tests in https://github.com/dotnet/runtime/tree/main/src/mono/mono/eventpipe/test that (AFAIK) don't run on any pipeline, but that is used for local validation of EP or container changes

…sources

… for runtimes that haven't adopted simdhash

…raries, and update CoreCLR to use the new model.

…rsion.

… there's very few reasons to use it in this way, but this is one of the scenarios for it).

…sources.

…ild.

…will fix some of the NativeAOT warnings

lambdageek · 2024-06-24T14:42:02Z

+#include <stdio.h>
+#include <stdlib.h>
+
+// Define a default implementation of the assert failure function.


Should we put something like this in minipal? it seems generally useful

I think I'd like to see more places where we'd use it before I move it to minipal. We can extract it the next time we'd have a use for it.

jkoritzinsky · 2024-06-24T20:42:20Z

/ba-g Wasm firefox timeout seems unrelated.

jkoritzinsky · 2024-06-08T00:25:25Z

Today, the src/native/containers and src/native/eventpipe libraries are defined in terms of source and header file lists.

This provides flexibility into how they're defined in CMake, but it makes it difficult to flow include directories or dependencies. It also makes it difficult to encapsulate logic, as these lists are defined in .cmake files that are included into a CMakeLists.txt file.

This PR converts these libraries to be defined in terms of CMake targets.

src/native/containers is transformed into an object library, so we can build it once and then link it into static libs, shared libs, or executables. An extern "C" block was added around all declarations that are currently used in eventpipe/diagnosticserver (so not dn-simdhash) to enable the build to build the containers library as a C library. Also, a default implementation of the dn_simdhash_assert_fail method was added to enable the containers lib to link into runtimes that don't currently provide an implementation.

src/native/eventpipe is transformed into interface libraries. Due to how eventpipe and diagnosticserver both require runtime-specific headers and need to be built with different flags for each target, defining them as interface libraries is the cleanest manner to handle this scenario (until we can refactor the ep-rt-*.h and ds-rt-*.h headers into a cleaner design).

This PR also updates the eventpipe and diagnosticserver codebases to use CMake's UNITY_BUILD support in each runtime's implementation instead of manually implementing a unity build.

dotnet-policy-service · 2024-06-08T00:26:00Z

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

lambdageek · 2024-06-10T18:51:00Z

FYI there's some eventpipe and containers unit tests in https://github.com/dotnet/runtime/tree/main/src/mono/mono/eventpipe/test that (AFAIK) don't run on any pipeline, but that is used for local validation of EP or container changes

…sources

… for runtimes that haven't adopted simdhash

…raries, and update CoreCLR to use the new model.

…rsion.

… there's very few reasons to use it in this way, but this is one of the scenarios for it).

…sources.

…ild.

…will fix some of the NativeAOT warnings

lambdageek · 2024-06-24T14:42:02Z

+#include <stdio.h>
+#include <stdlib.h>
+
+// Define a default implementation of the assert failure function.


Should we put something like this in minipal? it seems generally useful

I think I'd like to see more places where we'd use it before I move it to minipal. We can extract it the next time we'd have a use for it.

jkoritzinsky · 2024-06-24T20:42:20Z

/ba-g Wasm firefox timeout seems unrelated.

jkoritzinsky · 2024-06-08T00:25:25Z

Today, the src/native/containers and src/native/eventpipe libraries are defined in terms of source and header file lists.

This provides flexibility into how they're defined in CMake, but it makes it difficult to flow include directories or dependencies. It also makes it difficult to encapsulate logic, as these lists are defined in .cmake files that are included into a CMakeLists.txt file.

This PR converts these libraries to be defined in terms of CMake targets.

src/native/containers is transformed into an object library, so we can build it once and then link it into static libs, shared libs, or executables. An extern "C" block was added around all declarations that are currently used in eventpipe/diagnosticserver (so not dn-simdhash) to enable the build to build the containers library as a C library. Also, a default implementation of the dn_simdhash_assert_fail method was added to enable the containers lib to link into runtimes that don't currently provide an implementation.

src/native/eventpipe is transformed into interface libraries. Due to how eventpipe and diagnosticserver both require runtime-specific headers and need to be built with different flags for each target, defining them as interface libraries is the cleanest manner to handle this scenario (until we can refactor the ep-rt-*.h and ds-rt-*.h headers into a cleaner design).

This PR also updates the eventpipe and diagnosticserver codebases to use CMake's UNITY_BUILD support in each runtime's implementation instead of manually implementing a unity build.

dotnet-policy-service · 2024-06-08T00:26:00Z

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

lambdageek · 2024-06-10T18:51:00Z

FYI there's some eventpipe and containers unit tests in https://github.com/dotnet/runtime/tree/main/src/mono/mono/eventpipe/test that (AFAIK) don't run on any pipeline, but that is used for local validation of EP or container changes

…sources

… for runtimes that haven't adopted simdhash

…raries, and update CoreCLR to use the new model.

…rsion.

… there's very few reasons to use it in this way, but this is one of the scenarios for it).

…sources.

…ild.

…will fix some of the NativeAOT warnings

lambdageek · 2024-06-24T14:42:02Z

+#include <stdio.h>
+#include <stdlib.h>
+
+// Define a default implementation of the assert failure function.


Should we put something like this in minipal? it seems generally useful

I think I'd like to see more places where we'd use it before I move it to minipal. We can extract it the next time we'd have a use for it.

jkoritzinsky · 2024-06-24T20:42:20Z

/ba-g Wasm firefox timeout seems unrelated.

jkoritzinsky · 2024-06-08T00:25:25Z

Today, the src/native/containers and src/native/eventpipe libraries are defined in terms of source and header file lists.

This provides flexibility into how they're defined in CMake, but it makes it difficult to flow include directories or dependencies. It also makes it difficult to encapsulate logic, as these lists are defined in .cmake files that are included into a CMakeLists.txt file.

This PR converts these libraries to be defined in terms of CMake targets.

src/native/containers is transformed into an object library, so we can build it once and then link it into static libs, shared libs, or executables. An extern "C" block was added around all declarations that are currently used in eventpipe/diagnosticserver (so not dn-simdhash) to enable the build to build the containers library as a C library. Also, a default implementation of the dn_simdhash_assert_fail method was added to enable the containers lib to link into runtimes that don't currently provide an implementation.

src/native/eventpipe is transformed into interface libraries. Due to how eventpipe and diagnosticserver both require runtime-specific headers and need to be built with different flags for each target, defining them as interface libraries is the cleanest manner to handle this scenario (until we can refactor the ep-rt-*.h and ds-rt-*.h headers into a cleaner design).

This PR also updates the eventpipe and diagnosticserver codebases to use CMake's UNITY_BUILD support in each runtime's implementation instead of manually implementing a unity build.

dotnet-policy-service · 2024-06-08T00:26:00Z

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

lambdageek · 2024-06-10T18:51:00Z

FYI there's some eventpipe and containers unit tests in https://github.com/dotnet/runtime/tree/main/src/mono/mono/eventpipe/test that (AFAIK) don't run on any pipeline, but that is used for local validation of EP or container changes

…sources

… for runtimes that haven't adopted simdhash

…raries, and update CoreCLR to use the new model.

…rsion.

… there's very few reasons to use it in this way, but this is one of the scenarios for it).

…sources.

…ild.

…will fix some of the NativeAOT warnings

lambdageek · 2024-06-24T14:42:02Z

+#include <stdio.h>
+#include <stdlib.h>
+
+// Define a default implementation of the assert failure function.


Should we put something like this in minipal? it seems generally useful

I think I'd like to see more places where we'd use it before I move it to minipal. We can extract it the next time we'd have a use for it.

jkoritzinsky · 2024-06-24T20:42:20Z

/ba-g Wasm firefox timeout seems unrelated.

jkoritzinsky · 2024-06-08T00:25:25Z

Today, the src/native/containers and src/native/eventpipe libraries are defined in terms of source and header file lists.

This provides flexibility into how they're defined in CMake, but it makes it difficult to flow include directories or dependencies. It also makes it difficult to encapsulate logic, as these lists are defined in .cmake files that are included into a CMakeLists.txt file.

This PR converts these libraries to be defined in terms of CMake targets.

src/native/containers is transformed into an object library, so we can build it once and then link it into static libs, shared libs, or executables. An extern "C" block was added around all declarations that are currently used in eventpipe/diagnosticserver (so not dn-simdhash) to enable the build to build the containers library as a C library. Also, a default implementation of the dn_simdhash_assert_fail method was added to enable the containers lib to link into runtimes that don't currently provide an implementation.

src/native/eventpipe is transformed into interface libraries. Due to how eventpipe and diagnosticserver both require runtime-specific headers and need to be built with different flags for each target, defining them as interface libraries is the cleanest manner to handle this scenario (until we can refactor the ep-rt-*.h and ds-rt-*.h headers into a cleaner design).

This PR also updates the eventpipe and diagnosticserver codebases to use CMake's UNITY_BUILD support in each runtime's implementation instead of manually implementing a unity build.

dotnet-policy-service · 2024-06-08T00:26:00Z

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

lambdageek · 2024-06-10T18:51:00Z

FYI there's some eventpipe and containers unit tests in https://github.com/dotnet/runtime/tree/main/src/mono/mono/eventpipe/test that (AFAIK) don't run on any pipeline, but that is used for local validation of EP or container changes

…sources

… for runtimes that haven't adopted simdhash

…raries, and update CoreCLR to use the new model.

…rsion.

… there's very few reasons to use it in this way, but this is one of the scenarios for it).

…sources.

…ild.

…will fix some of the NativeAOT warnings

lambdageek · 2024-06-24T14:42:02Z

+#include <stdio.h>
+#include <stdlib.h>
+
+// Define a default implementation of the assert failure function.


Should we put something like this in minipal? it seems generally useful

I think I'd like to see more places where we'd use it before I move it to minipal. We can extract it the next time we'd have a use for it.

jkoritzinsky · 2024-06-24T20:42:20Z

/ba-g Wasm firefox timeout seems unrelated.

jkoritzinsky added EventPipe area-Infrastructure labels Jun 8, 2024

dotnet-policy-serviceBot assigned jkoritzinsky Jun 8, 2024

jkoritzinsky added 20 commits June 14, 2024 14:49

Move containers library to be an object library instead of a list of …
8a3bb1c
…sources

Define default implementation of the simdhash assert failure function…
0877350
… for runtimes that haven't adopted simdhash

Convert EventPipe and DiagnosticServer to be defined as interface lib…
3adeb0f
…raries, and update CoreCLR to use the new model.

Move NativeAOT over to the new model as well.
39118f0

Move Mono to the new eventpipe model
b7e9e82

Remove old eventpipe model
82562ff

Use CMake's Unity Build support instead of our own hacked-together ve…
7332da4
…rsion.

Flow dn_containers dependency
2f8f361

Add a comment to explain the rationale for using an INTERFACE lib (as…
bf6bb47
… there's very few reasons to use it in this way, but this is one of the scenarios for it).

PCH and UNITY_BUILD don't work together well
062aa32

Don't build NativeAOT's EP runtime as a unity build, only the shared …
e32ce79
…sources.

Add another dependency for dn-containers to try to fix the android bu…
f428cee
…ild.

Make the EXTERN_C_* macros into function-style macros
ed2a27e

Namespace-scope our extern-c block define
e6334b7

Reference diagnosticserver pal in NativeAOT.
847100d

Fix Windows build in debug-pal and fix NativeAOT linkage issues.
4969670

Correctly nest initializer for GCC
292fac5

Just directly extern "C" the headers
4d681a5

if not ifdef
81a686e

Try more directly including the container objects
5c13647

jkoritzinsky force-pushed the src-native-cmake-as-targets branch from 637d77f to 5c13647Compare June 14, 2024 21:58

Provide a no-lto flavor of the container library for NativeAOT. This …
a0fb40b
…will fix some of the NativeAOT warnings

This was referenced Jun 19, 2024
Long running System.ComponentModel tests #103265
Closed
LibraryTests (mostly) timing out #103674
Closed
STJ NullPropertyNameFail test failing in CI #103715
Closed

jkoritzinsky added 2 commits June 19, 2024 15:34

Make the default impl also cdecl
78739e3

Explicitly add eventpipe runtime objects to the static component library
6d2b05f

build-analysisBot mentioned this pull request Jun 20, 2024
[Mono AOT] Failed to compile System.Private.CoreLib.dll.bc #103520
Open

Install compile PDBs for each of the EP libs used in NativeAOT
d4ed370

This was referenced Jun 20, 2024
[Test Failure] System.Net.Http.WinHttpHandlerFunctional.Tests.BidirectionStreamingTest.BackwardsCompatibility_DowngradeToHttp11 #103754
Closed
System.Text.Json.Nodes.Tests.JsonObjectTests.NullPropertyNameFail [FAIL] #103784
Closed

jkoritzinsky marked this pull request as ready for review June 21, 2024 21:31

jkoritzinsky requested review from MichalStrehovsky, lambdageek, lateralusX, marek-safar and steveisok as code owners June 21, 2024 21:31

jkoritzinsky requested a review from a teamJune 21, 2024 21:32

lambdageek approved these changes Jun 24, 2024
View reviewed changes

Fix MSBuild build
fc10257

jkoritzinsky merged commit 71ab8f1 into dotnet:mainJun 24, 2024

jkoritzinsky deleted the src-native-cmake-as-targets branch June 24, 2024 20:43

This was referenced Jun 24, 2024
Failed USB connection via port 54050, error 61, in tvOS arm64 Release AllSubsets_Mono #82637
Closed
System.Buffers.Tests timeout on wasm MT #102040
Open
[browser] test failure: Tests timed out. Killing driver service #103524
Open

kotlarmilos mentioned this pull request Jun 25, 2024
Runtime tests fail to AOT compile on Apple mobile platforms #103982
Closed

matouskozak mentioned this pull request Jun 26, 2024
[Perf] Windows/x64: 8 Regressions on 6/26/2024 7:59:52 AM dotnet/perf-autofiling-issues#37081
Closed

tmds mentioned this pull request Jun 26, 2024
BIGENDIAN not defined while building eventpipe on s390x #104036
Closed

matouskozak mentioned this pull request Jul 4, 2024
Apple mobile JIT_Directed runtime tests failures: System.DllNotFoundException: EmptyStructsLib #104430
Closed

github-actionsBot locked and limited conversation to collaborators Jul 25, 2024