Enable EventPipe when EventSource is enabled

[LakshanF]

Fixes issue #81943. CrossGen2 has EventSourceSupport enabled and needs the EventPipe library in Windows.

[MichalStrehovsky]

Circling back to the discussion in #80382 (comment).

If EventSource doesn't work without enabling event pipe, shouldn't EventSourceSupport then enable event pipe? It doesn't look right that <EventSourceSupport>true</EventSourceSupport> is producing broken apps.

[ghost]

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

Issue Details

---

Fixes issue #81943. CrossGen2 has EventSourceSupport enabled and needs the EventPipe library in Windows.

Author:	LakshanF
Assignees:	LakshanF
Labels:	area-NativeAOT-coreclr
Milestone:	-

[MichalStrehovsky]

The libraries tests that are broken since eventpipe merged also enable eventsource so it's probably the same issue:

#82022 (comment)

I'd like to do something with those failing tests because it's noise and we tend to regress things worse once there's noise (old issue) that we can't distinguish from signal (newly introduced issue).

[LakshanF]

The libraries tests that are broken since eventpipe merged also enable eventsource so it's probably the same issue:

#82022 (comment)

I'd like to do something with those failing tests because it's noise and we tend to regress things worse once there's noise (old issue) that we can't distinguish from signal (newly introduced issue).

Created #82231 to track this

[MichalStrehovsky]

The libraries tests that are broken since eventpipe merged also enable eventsource so it's probably the same issue:
#82022 (comment)
I'd like to do something with those failing tests because it's noise and we tend to regress things worse once there's noise (old issue) that we can't distinguish from signal (newly introduced issue).

Created #82231 to track this

I'm looking at the reasoning there: "In NativeAOT, EventPipe library is not included by default in applications due to size considerations."

Here's how the numbers look like:

Default: 1,864,192
EventPipe: 1,928,192
EventSource: 3,277,824
EventSource+EventPipe: 3,342,848

I don't know how much bigger EventPipe is going to be, but even if it doubles in size (causing an almost 4% size increase for the EventSource enabled scenario), it might be the right tradeoff: simplifying the configuration matrix for users.

Cc @dotnet/ilc-contrib for thoughts

[agocke]

I'm looking at the reasoning there: "In NativeAOT, EventPipe library is not included by default in applications due to size considerations."

Could we leave this decision up to the managed linker? Is there a way to structure it such that you either use EventSource and EventPipe gets brought in automatically, or you don't and it's trimmed away?

[agocke]

I'd like to do something with those failing tests because it's noise and we tend to regress things worse once there's noise (old issue) that we can't distinguish from signal (newly introduced issue).

Yeah, we can't leave these tests failing for more than a day or two. @LakshanF if you can't get them to green soon we should revert the PR

[MichalStrehovsky]

Would defaulting EventPipe to enabled if EventSource is enabled on Windows fix both the tests and crossgen2? Based on the stack in #82022 (comment) it very well may.

We could then use #82231 for discussion whether that's the default we want to ship.

Managed trimming unfortunately can't get rid of EventSource even in a hello world because we statically use it in places - it's the reason why we need a feature switch for it.

[LakshanF]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[LakshanF]

Seems like the EventSource related tests are no longer failing in NativeAOT