From accc6eb0f0f669494bee0788884b5a10101f1243 Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Tue, 19 Mar 2019 14:46:18 -0700 Subject: [PATCH] Remove timer in RuntimeEventSource (#23311) * Change RuntimeEventSource's counter types * cleanup * cleanup --- .../Tracing/IncrementingEventCounter.cs | 2 +- .../Tracing/IncrementingPollingCounter.cs | 2 +- .../System/Diagnostics/Tracing/PollingCounter.cs | 2 +- .../Diagnostics/Eventing/RuntimeEventSource.cs | 97 ++++------------------ 4 files changed, 18 insertions(+), 85 deletions(-) diff --git a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/IncrementingEventCounter.cs b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/IncrementingEventCounter.cs index 1b83819..569c9a0 100644 --- a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/IncrementingEventCounter.cs +++ b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/IncrementingEventCounter.cs @@ -62,7 +62,7 @@ namespace System.Diagnostics.Tracing IncrementingCounterPayload payload = new IncrementingCounterPayload(); payload.Name = _name; payload.IntervalSec = intervalSec; - payload.DisplayName = (DisplayName == null) ? "" : DisplayName; + payload.DisplayName = DisplayName ?? ""; payload.DisplayRateTimeScale = (DisplayRateTimeScale == TimeSpan.Zero) ? "" : DisplayRateTimeScale.ToString("c"); payload.MetaData = GetMetaDataString(); payload.Increment = _increment - _prevIncrement; diff --git a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/IncrementingPollingCounter.cs b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/IncrementingPollingCounter.cs index 8cd5c42..b5a2582 100644 --- a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/IncrementingPollingCounter.cs +++ b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/IncrementingPollingCounter.cs @@ -71,7 +71,7 @@ namespace System.Diagnostics.Tracing { IncrementingCounterPayload payload = new IncrementingCounterPayload(); payload.Name = _name; - payload.DisplayName = (DisplayName == null) ? "" : DisplayName; + payload.DisplayName = DisplayName ?? ""; payload.DisplayRateTimeScale = (DisplayRateTimeScale == TimeSpan.Zero) ? "" : DisplayRateTimeScale.ToString("c"); payload.IntervalSec = intervalSec; payload.MetaData = GetMetaDataString(); diff --git a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/PollingCounter.cs b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/PollingCounter.cs index f156ef1..695b357 100644 --- a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/PollingCounter.cs +++ b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/PollingCounter.cs @@ -58,7 +58,7 @@ namespace System.Diagnostics.Tracing CounterPayload payload = new CounterPayload(); payload.Name = _name; - payload.DisplayName = (DisplayName == null) ? "" : DisplayName; + payload.DisplayName = DisplayName ?? ""; payload.Count = 1; // NOTE: These dumb-looking statistics is intentional payload.IntervalSec = intervalSec; payload.Mean = value; diff --git a/src/System.Private.CoreLib/src/System/Diagnostics/Eventing/RuntimeEventSource.cs b/src/System.Private.CoreLib/src/System/Diagnostics/Eventing/RuntimeEventSource.cs index 046d5f0..010c2ea 100644 --- a/src/System.Private.CoreLib/src/System/Diagnostics/Eventing/RuntimeEventSource.cs +++ b/src/System.Private.CoreLib/src/System/Diagnostics/Eventing/RuntimeEventSource.cs @@ -15,17 +15,11 @@ namespace System.Diagnostics.Tracing internal sealed class RuntimeEventSource : EventSource { private static RuntimeEventSource s_RuntimeEventSource; - private EventCounter[] _counters; - - private enum Counter { - GCHeapSize, - Gen0GCCount, - Gen1GCCount, - Gen2GCCount, - ExceptionCount - } - - private Timer _timer; + private PollingCounter _gcHeapSizeCounter; + private IncrementingPollingCounter _gen0GCCounter; + private IncrementingPollingCounter _gen1GCCounter; + private IncrementingPollingCounter _gen2GCCounter; + private IncrementingPollingCounter _exceptionCounter; private const int EnabledPollingIntervalMilliseconds = 1000; // 1 second @@ -36,84 +30,23 @@ namespace System.Diagnostics.Tracing private RuntimeEventSource(): base(new Guid(0x49592C0F, 0x5A05, 0x516D, 0xAA, 0x4B, 0xA6, 0x4E, 0x02, 0x02, 0x6C, 0x89), "System.Runtime", EventSourceSettings.EtwSelfDescribingEventFormat) { - } protected override void OnEventCommand(System.Diagnostics.Tracing.EventCommandEventArgs command) { if (command.Command == EventCommand.Enable) { - if (_counters == null) - { - // NOTE: These counters will NOT be disposed on disable command because we may be introducing - // a race condition by doing that. We still want to create these lazily so that we aren't adding - // overhead by at all times even when counters aren't enabled. - _counters = new EventCounter[] { - // TODO: process info counters - - // GC info counters - new EventCounter("Total Memory by GC", this), - new EventCounter("Gen 0 GC Count", this), - new EventCounter("Gen 1 GC Count", this), - new EventCounter("Gen 2 GC Count", this), - - new EventCounter("Exception Count", this) - }; - } - - - // Initialize the timer, but don't set it to run. - // The timer will be set to run each time PollForTracingCommand is called. - - // TODO: We should not need this timer once we are done settling upon a high-level design for - // what EventCounter is capable of doing. Once that decision is made, we should be able to - // get rid of this. - if (_timer == null) - { - _timer = new Timer( - callback: new TimerCallback(PollForCounterUpdate), - state: null, - dueTime: Timeout.Infinite, - period: Timeout.Infinite, - flowExecutionContext: false); - } - // Trigger the first poll operation on when this EventSource is enabled - PollForCounterUpdate(null); - } - else if (command.Command == EventCommand.Disable) - { - if (_timer != null) - { - _timer.Change(Timeout.Infinite, Timeout.Infinite); // disable the timer from running until System.Runtime is re-enabled - } - } - } - - public void UpdateAllCounters() - { - // GC counters - _counters[(int)Counter.GCHeapSize].WriteMetric(GC.GetTotalMemory(false)); - _counters[(int)Counter.Gen0GCCount].WriteMetric(GC.CollectionCount(0)); - _counters[(int)Counter.Gen1GCCount].WriteMetric(GC.CollectionCount(1)); - _counters[(int)Counter.Gen2GCCount].WriteMetric(GC.CollectionCount(2)); - _counters[(int)Counter.ExceptionCount].WriteMetric(Exception.GetExceptionCount()); - } - - private void PollForCounterUpdate(object state) - { - // TODO: Need to confirm with vancem about how to do error-handling here. - // This disables to timer from getting rescheduled to run, which may or may not be - // what we eventually want. - - // Make sure that any transient errors don't cause the listener thread to exit. - try - { - UpdateAllCounters(); - - // Schedule the timer to run again. - _timer.Change(EnabledPollingIntervalMilliseconds, Timeout.Infinite); + // NOTE: These counters will NOT be disposed on disable command because we may be introducing + // a race condition by doing that. We still want to create these lazily so that we aren't adding + // overhead by at all times even when counters aren't enabled. + + // On disable, PollingCounters will stop polling for values so it should be fine to leave them around. + _gcHeapSizeCounter = _gcHeapSizeCounter ?? new PollingCounter("GC Heap Size", this, () => GC.GetTotalMemory(false)); + _gen0GCCounter = _gen0GCCounter ?? new IncrementingPollingCounter("Gen 0 GC Count", this, () => GC.CollectionCount(0)); + _gen1GCCounter = _gen1GCCounter ?? new IncrementingPollingCounter("Gen 1 GC Count", this, () => GC.CollectionCount(1)); + _gen2GCCounter = _gen2GCCounter ?? new IncrementingPollingCounter("Gen 2 GC Count", this, () => GC.CollectionCount(2)); + _exceptionCounter = _exceptionCounter ?? new IncrementingPollingCounter("Exception Count", this, () => Exception.GetExceptionCount()); } - catch { } } } } -- 2.7.4