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

EventPipe rundown happening at the end of a session #86103

Open
vaind opened this issue May 11, 2023 · 9 comments
Open

EventPipe rundown happening at the end of a session #86103

vaind opened this issue May 11, 2023 · 9 comments
Labels
area-Tracing-coreclr enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@vaind
Copy link

vaind commented May 11, 2023

I'm trying to implement continuous profiler with Diagnostics IPC StartEventPipeSession(). The idea is to start a profiler when the app launches and only do sample-profiling of select parts of the application. O

Originally, I've tried doing this by creating a new EventPipeSession each time I needed to profile some time span but the startup is pretty heavy (about 100 ms to start when the Microsoft-DotNETCore-SampleProfiler provider is included in the list). Therefore, I've switched to starting the session at the beginning and subscribing to the SampleProfilerTraceEventParser.ThreadSample only for the time needed. This way, the overall overhead is reasonable (below 5 % from my testing) and the long session startup time doesn't affect actual operations, only the app startup (and can be made in the background).

Now to the issue at hand

As the samples are captured, I'm trying to assign modules and methods. These are, however, only provided by the rundown provider when the session is being stopped. In my use-case, this would mean when the application is shut down. The relevant code is:

// Do rundown before fully stopping the session unless rundown wasn't requested
if (ep_session_get_rundown_requested (session) && _ep_can_start_threads) {
ep_session_enable_rundown (session); // Set Rundown provider.
EventPipeThread *const thread = ep_thread_get_or_create ();
if (thread != NULL) {
ep_thread_set_as_rundown_thread (thread, session);
{
config_enable_disable (ep_config_get (), session, provider_callback_data_queue, true);
{
ep_session_execute_rundown (session, _ep_rundown_execution_checkpoints);
}
config_enable_disable(ep_config_get (), session, provider_callback_data_queue, false);
}
ep_thread_set_as_rundown_thread (thread, NULL);
} else {
EP_ASSERT (!"Failed to get or create the EventPipeThread for rundown events.");
}
}

I wonder what the reason for this is and whether there is a way to trigger rundown right at the beginning of the session and then rely on runtime provider during the application lifetime to get updates.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label May 11, 2023
@ghost
Copy link

ghost commented May 11, 2023

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

I'm trying to implement continuous profiler with Diagnostics IPC StartEventPipeSession(). The idea is to start a profiler when the app launches and only do sample-profiling of select parts of the application. O

Originally, I've tried doing this by creating a new EventPipeSession each time I needed to profile some time span but the startup is pretty heavy (about 100 ms to start when the Microsoft-DotNETCore-SampleProfiler provider is included in the list). Therefore, I've switched to starting the session at the beginning and subscribing to the SampleProfilerTraceEventParser.ThreadSample only for the time needed. This way, the overall overhead is reasonable (below 5 % from my testing) and the long session startup time doesn't affect actual operations, only the app startup (and can be made in the background).

Now to the issue at hand

As the samples are captured, I'm trying to assign modules and methods. These are, however, only provided by the rundown provider when the session is being stopped. In my use-case, this would mean when the application is shut down. The relevant code is:

// Do rundown before fully stopping the session unless rundown wasn't requested
if (ep_session_get_rundown_requested (session) && _ep_can_start_threads) {
ep_session_enable_rundown (session); // Set Rundown provider.
EventPipeThread *const thread = ep_thread_get_or_create ();
if (thread != NULL) {
ep_thread_set_as_rundown_thread (thread, session);
{
config_enable_disable (ep_config_get (), session, provider_callback_data_queue, true);
{
ep_session_execute_rundown (session, _ep_rundown_execution_checkpoints);
}
config_enable_disable(ep_config_get (), session, provider_callback_data_queue, false);
}
ep_thread_set_as_rundown_thread (thread, NULL);
} else {
EP_ASSERT (!"Failed to get or create the EventPipeThread for rundown events.");
}
}

I wonder what the reason for this is and whether there is a way to trigger rundown right at the beginning of the session and then rely on runtime provider during the application lifetime to get updates.

Author: vaind
Assignees: -
Labels:

area-Diagnostics-coreclr

Milestone: -

@tommcdon tommcdon added the question Answer questions and provide assistance, not an issue with source code or documentation. label May 11, 2023
@tommcdon tommcdon added this to the 8.0.0 milestone May 11, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label May 11, 2023
@tommcdon
Copy link
Member

@davmason @noahfalk

@davmason
Copy link
Member

Hi @vaind,

To answer the question of why rundown behaves this way, it's because it was designed for the scenario where you take a trace and then do analysis later. It wasn't designed to do live processing and analysis of traces. This dates back many years to the desktop framework.

Having rundown happen at the beginning of the session is something we have thought about but so far other work items have taken priority.

For your scenario, a trace will continue to emit method and module events as they are loaded, so you could always work around by starting two sessions and immediately stopping one. The stopped session will give you rundown and then you can get future events from the ongoing session.

@vaind
Copy link
Author

vaind commented May 12, 2023

Thanks @davmason, that's what I've assumed. I've also thought about that workaround but wasn't sure it would be alright and wouldn't miss anything important. Thanks for the confirmation it should work that way.

As you're saying you've thought about having rundown at the beginning of the session (or maybe it could be triggered by an EventPipeSession method instead to provide greater flexibility), I guess this issue can stay open as a feature request?

@hoyosjs hoyosjs added the enhancement Product code improvement that does NOT require public API changes/additions label May 15, 2023
@tommcdon tommcdon removed the question Answer questions and provide assistance, not an issue with source code or documentation. label May 17, 2023
@tommcdon tommcdon modified the milestones: 8.0.0, Future May 17, 2023
@tommcdon
Copy link
Member

After careful consideration, we do not plan to action this particular item in this release. We will continue to evaluate it for future releases. Ideally, we would like to fix every issue and implement every idea people submit. Realistically, we cannot address every item.

@vaind
Copy link
Author

vaind commented May 18, 2023

Thanks for the update. Looking forward to this, whenever it does make the cut.

@brianrob
Copy link
Member

Given the streaming scenario for EventPipe, it would be good to keep this around and implement it in the future. I don't imagine that it would take too much work to make it happen - likely just updating the callbacks in the runtime to allow EventPipe sources to trigger rundown (both start and stop). Not sure what the corresponding change in Mono would be, but @lateralusX will know.

@bruno-garcia
Copy link
Member

@brianrob with the move towards NativeAOT, do we need to change Mono too?

.NET 8 is out, any chance this can make it to .NET 9?

@brianrob
Copy link
Member

brianrob commented Dec 5, 2023

I anticipate that if we're going to make this change in CoreCLR, we'd want to do so in Mono too if for no other reason than consistency. I don't think that NativeAOT affects anything here.

In terms of getting this into .NET 9, I will defer to @davmason and @tommcdon who own this space for CoreCLR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Tracing-coreclr enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

6 participants