Move EventPipe C library into shared location.

[lateralusX]

Move C version of EventPipe library into a folder that can be shared by both CoreClr and Mono:

src/native/eventpipe

All runtime specific EventPipe files (shim implementation, build files etc) will be kept under each runtime (mono/mono/eventpipe for example). Example on how shared EventPipe sources will be consumed by mono build, src/mono/mono/eventpipe/CMakeLists.txt.

PR also adds ability to build mono/mono/eventpipe/test using CMake (needed after mono switched to cmake). Tests are still mono specific so will be kept there for now, but will most likely be shared between runtimes at a later point in time.

@jkotas @stephentoub Do you believe the location of new folder where we can share native code between runtimes is OK, src/native/eventpipe?

@lambdageek @josalem

[jkotas]

I am not sure whether we need the second src in the path (ie I would make it src/common/eventpipe). We have the second src in the path for coreclr because of we run out of time to remove it during repo consolidation. Neither src/libraries, src/mono, src/installer or src/tests have it.

[jkotas]

LGTM otherwise. Thank you!

[jkotas]

The name we have discussed before on Teams was src/shared. Either src/shared or src/common sound good to me. We have prior art for both.

[lateralusX]

I am not sure whether we need the second src in the path (ie I would make it src/common/eventpipe). We have the second src in the path for coreclr because of we run out of time to remove it during repo consolidation. Neither src/libraries, src/mono, src/installer or src/tests have it.

Totally agree, will drop the second src now when I know the background why we have it under coreclr (due to out of time removing it).

[jkotas]

Actually, I am thinking whether the best name for this would be src/native. It would be a directory to consolidate all native code (that is shared between runtimes) under.

For example, we can move the host under this directory from under installer where it does not really belong (related to #43700 - cc @VSadov @vitek-karas). It would look a bit odd for the host to be src/common/host. src/native/host looks better.

[safern]

In my opinion I like src/native better as we already have a managed common path for System.Private.CoreLib, which is shared across both runtimes.

Also we have System.Globalization.Native native shim under src/libraries/Native should we consider moving that to this new folder as well?

[jkotas]

With this plan, I think we can consider moving everything under src/libraries/native to src/native

[lateralusX]

Great, sounds like we align on src/native

I will do change tomorrow morning, so there is still some time to change our minds 😊.

[josalem]

Seconding src/native after reading the arguments for it.

CC @dotnet/dotnet-diag - this is a good PR for catching up on the EventPipe x Mono workstream. it highlights the shared components, but is also a good jump point for identifying the runtime specific code.

[lateralusX]

@josalem I have a separate PR coming up after this one, implementing the CoreClr shim + add it into CoreClr build (initially as an opt in feature). With that PR we can build CoreClr with either C++ or C version of EventPipe for sometime while transitioning over to new library.

[jkoritzinsky]

I also really like the src/native option.

[VSadov]

src/native sounds good.

As for the plan for the native libs - on an example of System.Globalization.Native, is it going into src/native directly or under src/native/libraries ?

I'd mildly prefer the src/native/libraries, for esthetic reasons.
I assume eventually all the native libs code ends up there. We have between 2 to 5 native libraries right now depending on platform, with some parts common and some platform specific. So it is likely to be a small tree with common parts closer to the root. Having that just under src/native could feel a bit messy.

I also assume we are not moving S.G.N right away, just thinking one step ahead?

[am11]

Common parts could also be plain code files (.c,.cpp) included by make file elsewhere in the tree. There was a wish to share some code files across varied (high-level) components of the repo. One example is a method to get executable full path, implementation of which is currently duplicated in corehost, coreclr and libSystem.Native for six different platforms. So perhaps src/native/common (or src/native/shared) could be a suitable place.

[lateralusX]

So for the eventpipe library (the one moved in this PR), should we do src/native/eventpipe or src/native/libraries/eventpipe ?

[VSadov]

By "native libraries" I mean native parts of FX libraries - like System.Globalization.Native, System.IO.Compression.Native and the like.

From that point eventpipe is a separate thing, so it would be src/native/eventpipe.

[jkotas]

src/native/common

Sounds good to me.

src/native/eventpipe or src/native/libraries/eventpipe

I would keep it as src/native/eventpipe

[jkotas]

We have between 2 to 5 native libraries right now depending on platform,
Having that just under src/native could feel a bit messy.

Why would it be messy? As you have said, it is a few different libraries. I do not see very fundamental difference between eventpipe and the other System.*.Native. One can think about eventpipe as System.Diagnostics.Tracing.Native.

In any case, we do not have to solve this now.

[VSadov]

We have native code common to libraries (like zlib) or to Unix only (i.e. PAL stuff) and thus the code is currently organized into a subtree like https://github.com/dotnet/runtime/tree/master/src/libraries/Native . It probably makes sense to keep it in that shape.
It can go directly under src/native too. It is mostly about "looks" not the functionality.

I am less familiar with eventpipe and whether it shares anything with that tree. I assumed it is a component more similar to hosting.

Yes, no need to solve this now. Not a big deal anyways.

[lateralusX]

Moved runtime agnostic C EventPipe/DiagnosticServer library to src/native/eventpipe

When all CI checks have passed I believe this PR is ready to go!

[jkotas]

Thank you!

[vitek-karas]

src/native/host for hosting components

I like this - would definitely make more sense than the current location.

Just curious as I didn't see this explicitly stated above - the plan is to move tests for the native event pipe under src/native as well, right?
What's the thinking on the test structure there: src/native/eventpipe/tests or src/native/tests/eventpipe, or something else?

It would also be good to think about what the tests are "Allowed" to do in this subtree - for example all the hosts tests are in C# (with only small parts in native). Maybe that's OK...

[lateralusX]

The tests I called out above is primarily low level native EventPipe unit tests developed when doing the transform of C++ CoreClr EventPipe/DiagnosticServer library into a shareable component. Those tests are currently hosted under src/mono/mono/eventpipe/test, since they are depending on mono eglib test runner and some mono runtime API's. My idea was to make them platform agnostic, so we can at least share the test code (might be a runtime specific piece and build connected to the test runner) somewhere under src/native as well, but that is for later.

[jkotas]

the plan is to move tests for the native event pipe under src/native as well, right?

I think src/tests should be the place for tests that are not simple API tests.

In fact, we do have a bunch of eventpipe tests there already: https://github.com/dotnet/runtime/tree/master/src/tests/tracing/eventpipe .

[lateralusX]

Jupp there are a bunch of managed EventPipe/DiagnosticServer tests under src/tests.