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.

Sign up for GitHub

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

[Release/6.0] Port EventCounters multi session support to 6.0 #84680

Merged
merged 3 commits into from
May 11, 2023
Merged

[Release/6.0] Port EventCounters multi session support to 6.0 #84680

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1ead49f
Port 82970 to 6.0
davmason Apr 8, 2023
fb6a4af
Code Review feedback
davmason Apr 14, 2023
4a618fd
Make asserts match code after code review feedback
davmason May 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 33 additions & 11 deletions src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/CounterGroup.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,24 +52,46 @@ private void RegisterCommandCallback()

private void OnEventSourceCommand(object? sender, EventCommandEventArgs e)
{
if (e.Command == EventCommand.Enable || e.Command == EventCommand.Update)
{
Debug.Assert(e.Arguments != null);
// Should only be enable or disable
Debug.Assert(e.Command == EventCommand.Enable || e.Command == EventCommand.Disable);

if (e.Arguments.TryGetValue("EventCounterIntervalSec", out string? valueStr) && float.TryParse(valueStr, out float value))
lock (s_counterGroupLock) // Lock the CounterGroup
{
if (e.Command == EventCommand.Enable || e.Command == EventCommand.Update)
{
lock (s_counterGroupLock) // Lock the CounterGroup
Debug.Assert(e.Arguments != null);

if (!e.Arguments.TryGetValue("EventCounterIntervalSec", out string? valueStr)
|| !float.TryParse(valueStr, out float intervalValue))
{
EnableTimer(value);
// Command is Enable but no EventCounterIntervalSec arg so ignore
return;
}

EnableTimer(intervalValue);
}
}
else if (e.Command == EventCommand.Disable)
{
lock (s_counterGroupLock)
else if (e.Command == EventCommand.Disable)
{
DisableTimer();
// Since we allow sessions to send multiple Enable commands to update the interval, we cannot
// rely on ref counting to determine when to enable and disable counters. You will get an arbitrary
// number of Enables and one Disable per session.
//
// Previously we would turn off counters when we received any Disable command, but that meant that any
// session could turn off counters for all other sessions. To get to a good place we now will only
// turn off counters once the EventSource that provides the counters is disabled. We can then end up
// keeping counters on too long in certain circumstances - if one session enables counters, then a second
// session enables the EventSource but not counters we will stay on until both sessions terminate, even
// if the first session terminates first.
if (!_eventSource.IsEnabled())
{
DisableTimer();
}
}

Debug.Assert((s_counterGroupEnabledList == null && !_eventSource.IsEnabled())
|| (_eventSource.IsEnabled() && s_counterGroupEnabledList!.Contains(this))
|| (_pollingIntervalInMilliseconds == 0 && !s_counterGroupEnabledList!.Contains(this))
|| (!_eventSource.IsEnabled() && !s_counterGroupEnabledList!.Contains(this)));
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2733,9 +2733,6 @@ internal void DoCommand(EventCommandEventArgs commandArgs)
m_eventSourceEnabled = true;
}

this.OnEventCommand(commandArgs);
this.m_eventCommandExecuted?.Invoke(this, commandArgs);

if (!commandArgs.enable)
{
// If we are disabling, maybe we can turn on 'quick checks' to filter
Expand Down Expand Up @@ -2767,6 +2764,9 @@ internal void DoCommand(EventCommandEventArgs commandArgs)
m_eventSourceEnabled = false;
}
}

this.OnEventCommand(commandArgs);
this.m_eventCommandExecuted?.Invoke(this, commandArgs);
}
else
{
Expand Down