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

Initial work to port EventPipe library to NativeAOT #80382

Merged
merged 34 commits into from
Feb 8, 2023

Conversation

LakshanF
Copy link
Member

@LakshanF LakshanF commented Jan 9, 2023

Getting EventPipe to full functionality as outlined in #79241 still has significant work remaining, but this PR is at a sweet spot between demonstrating end-to-end event data flowing from a NativeAOT application to a listening client and having too much new code to CR.

The current status allows a NativeAOT application to send EventSource events to a listening client (command CollectTracing2 in https://github.com/dotnet/diagnostics/blob/main/documentation/design-docs/ipc-protocol.md) in windows. This is done by adding a new opt-in library for AOT.

Managed EventSource Support Native AOT Library NativeAOT EventSource Application
Disabled Disabled Same behavior as .NET 7
Enabled Enabled Managed EventSource data and native ProcessInfo events are received by the listening client
Disabled Enabled Native ProcessInfo event is received by the listening client
Enabled Disabled Not supported

The NativeAOT EventSource sample application is currently shown in reproNative and the listening client application is a .NET application similar to this client application.

New Code

Although the 'new code' in this initial PR seems large with 6000+, there is really not much new code that is getting checked in as explained below. As outlined in https://github.com/dotnet/runtime/tree/main/src/native/eventpipe#eventpipe-and-diagnosticserver, NativeAOT EventPipe library uses the common code that both Mono and CoreCLR runtime uses and provides the following features as required by the EventPipe library;

  • Container classes (this is currently being moved to common code. Specifically, when that happens, the container classes in this checkin will be removed). This checkin has around ~1000 lines of code for this. Issue Leverage common container code for NativeAOT EventPipe  #81002 tracks moving the container code to the pending common implementation in the EventPipe library.
  • Runtime functionality. NativeAOT runtime code started with a copy of CoreCLR code and these checkins in the PR show a way to more easily see the differences. See this commit to see specific differences from the CoreCLR implementation easily. The changes here has around ~5000 lines of code.

The rest of the new code is mostly as below:

  • Hooks between Managed and Native code to enable EventSource events to flow
  • Some changes in the NativeAOT runtime side to support EventPipe
  • Opt-in EventPipe library that contains hookup code from Startup.cpp to the EventPipe library (Aot.EventPipe.lib which is used when EventSource is enabled in NativeAOT and Aot.Disabled.EventPipe.lib when EventSource is disabled). This commit shows details
  • Changes to EventPipe common code
  • Build artifacts
  • A native sample application with EventSource to demonstrate EventPipe. A client application is used to test that the NativeAOT application can send event data through the EventPipe.

@ghost
Copy link

ghost commented Jan 9, 2023

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

Getting EventPipe to full functionality as outlined in #79241 still has significant work remaining, but this PR is at a sweet spot between demonstrating end-to-end event data flowing from a NativeAOT application to a listening client and having too much new code to CR.

The current status allows a NativeAOT application to send EventSource events to a listening client (command CollectTracing2 in https://github.com/dotnet/diagnostics/blob/main/documentation/design-docs/ipc-protocol.md) in windows. This is done by adding a new opt-in library for AOT.

Managed EventSource Support Native AOT Library NativeAOT EventSource Application
Disabled Disabled Same behavior as .NET 7
Enabled Enabled Managed EventSource data and native ProcessInfo events are received by the listening client
Disabled Enabled Native ProcessInfo event is received by the listening client
Enabled Disabled Not supported

The NativeAOT EventSource sample application is currently shown in reproNative and the listening client application is a .NET application similar to https://github.com/LakshanF/CSharp/blob/master/src/aot/experiments/Diagnostics/Logging/EventSource/DiagnsoticsClient/Program.cs

New Code

Although the 'new code' in this initial PR seems large with 6000+, there is really not much new code that is getting checked in as explained below. As outlined in https://github.com/dotnet/runtime/tree/main/src/native/eventpipe#eventpipe-and-diagnosticserver, NativeAOT EventPipe library uses the common code that both Mono and CoreCLR runtime uses and provides the following features as required by the EventPipe library;

  • Container classes (this is currently being moved to common code. Specifically, when that happens, the container classes in this checkin will be removed). This checkin has around ~1000 lines of code for this.
  • Runtime functionality. NativeAOT runtime code started with a copy of CoreCLR code and these checkins in the PR show a way to more easily see the differences. See this commit to see specific differences from the CoreCLR implementation easily. The changes here has around ~5000 lines of code.

The rest of the new code is mostly as below:

  • Hooks between Managed and Native code to enable EventSource events to flow
  • Some changes in the NativeAOT runtime side to support EventPipe
  • Opt-in EventPipe library that contains hookup code from Startup.cpp to the EventPipe library (Aot.EventPipe.lib which is used when EventSource is enabled in NativeAOT and Aot.Disabled.EventPipe.lib when EventSource is disabled). This commit shows details
  • Changes to EventPipe common code
  • Build artifacts
  • A native sample application with EventSource to demonstrate EventPipe. This is not meant to be checked in. A client application similar to this can be used to test.
Author: LakshanF
Assignees: -
Labels:

area-NativeAOT-coreclr

Milestone: -

Copy link
Member

@MichalStrehovsky MichalStrehovsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just flushing out a couple comments I have. I'll clearly need to spend a lot more time on understanding all this.

…e.Windows.targets

Co-authored-by: Michal Strehovský <MichalStrehovsky@users.noreply.github.com>
@MichalStrehovsky
Copy link
Member

MichalStrehovsky@99a0dd3 has the general direction of how to resolve the incompatibilities between redhawk headers (that don't want to include Windows.h) and eventpipe (that wants Windows.h).

Basically, put everything that uses Redhawk headers into the .cpp file and not in the header. There's probably some minimal subset of redhawk headers that we want to include (like the atomics) so that these don't turn into function calls. But event pipe C sources should see as little of redhawk as possible. Putting redhawk headers into places that will make them included from event pipe is asking for trouble.

If needed we can refactor things in the redhawk headers like I did with the move of __PN__MACHINECALL_CDECL_OR_DEFAULT, but these should just be tiny fixes.

I don't think we should defer into #81285. That issue should be fixed with this PR. It's requiring us to do unnatural things and they'll have to be backed out.

@LakshanF LakshanF marked this pull request as ready for review February 7, 2023 11:50
@LakshanF
Copy link
Member Author

LakshanF commented Feb 7, 2023

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@runfoapp runfoapp bot mentioned this pull request Feb 7, 2023
* Undo unnecessary changes
* Make sure runtime can build without FEATURE_PERFTRACING
Copy link
Member

@MichalStrehovsky MichalStrehovsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed out two commits with cleanups, please have a look.

Otherwise looks reasonable. Would be cool if someone who can tell a difference between even pipe and diagnostic pipe can have a look as well.

@LakshanF LakshanF merged commit 081d93a into dotnet:main Feb 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants