From 2b3cf84359d03d3f7dedaad784588fd19ac63035 Mon Sep 17 00:00:00 2001 From: gmega Date: Fri, 1 Mar 2024 09:08:48 -0300 Subject: [PATCH] carve out a better defined external profiler API --- chroprof/api.nim | 24 ++++++++++++------------ chroprof/collector.nim | 14 +++++++------- chroprof/events.nim | 16 +++++++++++++++- chroprof/profiler.nim | 12 ++++++------ tests/testevents.nim | 5 ++--- tests/testmetricscollector.nim | 2 +- tests/testprofiler.nim | 5 ++--- tests/utils.nim | 15 +++++---------- 8 files changed, 50 insertions(+), 43 deletions(-) diff --git a/chroprof/api.nim b/chroprof/api.nim index af24f8a..d7257cf 100644 --- a/chroprof/api.nim +++ b/chroprof/api.nim @@ -1,11 +1,8 @@ -import chronos/futures import ./[profiler, events] export Event, ExtendedFutureState, ProfilerState, MetricsTotals, AggregateMetrics, FutureType, execTimeWithChildren -type EventCallback* = proc (e: Event) {.nimcall, gcsafe, raises: [].} - var profilerInstance {.threadvar.}: ProfilerState proc getMetrics*(): MetricsTotals = @@ -13,13 +10,16 @@ proc getMetrics*(): MetricsTotals = ## current thread. result = profilerInstance.metrics -proc enableEventCallbacks*(callback: EventCallback): void = - onBaseFutureEvent = handleBaseFutureEvent - onAsyncFutureEvent = handleAsyncFutureEvent - handleFutureEvent = callback - -proc enableProfiling*(clientCallback: EventCallback = nil) = +proc enableProfiling*(callback: EventCallback = nil) = ## Enables profiling for the the event loop running in the current thread. - handleFutureEvent = proc (e: Event) {.nimcall.} = - profilerInstance.processEvent(e) - if not isNil(clientCallback): clientCallback(e) + ## The client may optionally supply a callback to be notified of `Future` + ## events. + enableMonitoring( + if (isNil(callback)): + proc(e: Event) {.nimcall.} = + profilerInstance.processEvent(e) + else: + proc (e: Event) {.nimcall.} = + profilerInstance.processEvent(e) + callback(e) + ) diff --git a/chroprof/collector.nim b/chroprof/collector.nim index 9e65926..714f7ed 100644 --- a/chroprof/collector.nim +++ b/chroprof/collector.nim @@ -15,7 +15,7 @@ import ./api when defined(metrics): type ChronosProfilerInfo* = ref object of RootObj - perfSampler: PerfSampler + sampler: MetricsSampler sampleInterval: times.Duration clock: Clock k: int @@ -23,7 +23,7 @@ when defined(metrics): lastSample: Time collections*: uint - PerfSampler = proc (): MetricsTotals {.raises: [].} + MetricsSampler = proc (): MetricsTotals {.raises: [].} Clock = proc (): Time {.raises: [].} @@ -76,12 +76,12 @@ when defined(metrics): proc newCollector*( ChronosProfilerInfo: typedesc, - perfSampler: PerfSampler, + sampler: MetricsSampler, clock: Clock, sampleInterval: times.Duration, k: int = 10, ): ChronosProfilerInfo = ChronosProfilerInfo( - perfSampler: perfSampler, + sampler: sampler, clock: clock, k: k, sampleInterval: sampleInterval, @@ -137,7 +137,7 @@ when defined(metrics): self.collections += 1 var currentMetrics = self. - perfSampler(). + sampler(). pairs. toSeq. # We don't scoop metrics with 0 exec time as we have a limited number of @@ -179,7 +179,7 @@ when defined(metrics): " the one that initialized the metricscolletor module." asyncProfilerInfo = ChronosProfilerInfo.newCollector( - perfSampler = proc (): MetricsTotals = getMetrics(), + sampler = getMetrics, k = k, # We want to collect metrics every 5 seconds. sampleInterval = initDuration(seconds = 5), @@ -187,7 +187,7 @@ when defined(metrics): ) enableProfiling( - proc (e: Event) {.nimcall, gcsafe.} = + proc (e: Event) {.nimcall, gcsafe, raises: [].} = {.cast(gcsafe).}: if e.newState == ExtendedFutureState.Completed: asyncProfilerInfo.collect() diff --git a/chroprof/events.nim b/chroprof/events.nim index a2ecfe6..5ac23f2 100644 --- a/chroprof/events.nim +++ b/chroprof/events.nim @@ -17,11 +17,14 @@ type Failed, Event* = object + ## A timestamped event transition in a `Future` state. future: FutureBase newState*: ExtendedFutureState timestamp*: Moment + + EventCallback* = proc (e: Event) {.nimcall, gcsafe, raises: [].} -var handleFutureEvent* {.threadvar.}: proc (event: Event) {.nimcall, gcsafe, raises: [].} +var handleFutureEvent {.threadvar.}: EventCallback proc `location`*(self: Event): SrcLoc = self.future.internalLocation[Create][] @@ -58,5 +61,16 @@ proc handleAsyncFutureEvent*(future: FutureBase, if not isNil(handleFutureEvent): handleFutureEvent(mkEvent(future, extendedState)) +proc enableMonitoring*(callback: EventCallback) = + ## Enables monitoring of Chronos `Future` state transitions on the + ## event loop that runs in the current thread. The provided callback will be + ## called at every such event. + onBaseFutureEvent = handleBaseFutureEvent + onAsyncFutureEvent = handleAsyncFutureEvent + handleFutureEvent = callback +proc stopMonitoring*() = + onBaseFutureEvent = nil + onAsyncFutureEvent = nil + handleFutureEvent = nil diff --git a/chroprof/profiler.nim b/chroprof/profiler.nim index 4201651..f1c55a1 100644 --- a/chroprof/profiler.nim +++ b/chroprof/profiler.nim @@ -1,7 +1,7 @@ ## This module contains the actual profiler implementation - the piece of code -## responsible for computing and aggregating metrics from timestamped event -## sequences. -## +## responsible for computing metrics from sequences of timestamped events and +## aggregating them. + import std/[tables, options, sets] import chronos/[timer, srcloc] @@ -42,7 +42,7 @@ type callCount*: uint ## Total number of distinct `Future`s observed ## for this `FutureType`. - PartialMetrics* = object + PartialMetrics = object state*: ExtendedFutureState created*: Moment lastStarted*: Moment @@ -53,7 +53,7 @@ type parent*: Option[uint] pauses*: uint - MetricsTotals* = Table[SrcLoc, AggregateMetrics] + MetricsTotals* = Table[FutureType, AggregateMetrics] ProfilerState* = object callStack: seq[uint] @@ -152,7 +152,7 @@ proc futureCompleted(self: var ProfilerState, event: Event): void = self.partials.del(event.futureId) -proc processEvent*(self: var ProfilerState, event: Event): void = +proc processEvent*(self: var ProfilerState, event: Event): void {.nimcall, gcsafe, raises: []} = case event.newState: of Pending: self.futureCreated(event) of Running: self.futureRunning(event) diff --git a/tests/testevents.nim b/tests/testevents.nim index 287e2c3..ed23e0c 100644 --- a/tests/testevents.nim +++ b/tests/testevents.nim @@ -9,11 +9,10 @@ import ./utils suite "event ordering expectations": setup: - installCallbacks() + startRecording() teardown: - clearRecording() - revertCallbacks() + stopRecording() test "should emit correct events for a simple future": diff --git a/tests/testmetricscollector.nim b/tests/testmetricscollector.nim index 104ee38..99a7199 100644 --- a/tests/testmetricscollector.nim +++ b/tests/testmetricscollector.nim @@ -61,7 +61,7 @@ suite "metrics collector": proc setupCollector(k: int = high(int)): void = collector = ChronosProfilerInfo.newCollector( - perfSampler = proc (): MetricsTotals = sample, + sampler = proc (): MetricsTotals = sample, clock = proc (): Time = wallTime, sampleInterval = times.initDuration(minutes = 5), k = k, diff --git a/tests/testprofiler.nim b/tests/testprofiler.nim index 7d23022..0d9dcc3 100644 --- a/tests/testprofiler.nim +++ b/tests/testprofiler.nim @@ -11,11 +11,10 @@ import ./utils suite "profiler metrics": setup: - installCallbacks() + startRecording() teardown: - clearRecording() - revertCallbacks() + stopRecording() resetTime() proc recordedMetrics(): MetricsTotals = diff --git a/tests/utils.nim b/tests/utils.nim index 18011c5..8bbd124 100644 --- a/tests/utils.nim +++ b/tests/utils.nim @@ -30,19 +30,14 @@ proc recordSegment*(segment: string) = state: ExtendedFutureState.Running )) -proc clearRecording*(): void = +proc stopRecording*(): void = recording = @[] rawRecording = @[] + stopMonitoring() -proc installCallbacks*() = - assert isNil(handleFutureEvent), "There is a callback already installed" - - enableEventCallbacks(recordEvent) - -proc revertCallbacks*() = - assert not isNil(handleFutureEvent), "There are no callbacks installed" - - handleFutureEvent = nil +proc startRecording*() = + stopRecording() + enableMonitoring(recordEvent) proc forProc*(self: var MetricsTotals, procedure: string): AggregateMetrics = for (key, aggMetrics) in self.mpairs: