Add global worker statistics param (QUIC_PARAM_GLOBAL_WORKER_STATISTICS)#6115
Add global worker statistics param (QUIC_PARAM_GLOBAL_WORKER_STATISTICS)#6115guhetier wants to merge 6 commits into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (60.78%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #6115 +/- ##
==========================================
+ Coverage 85.13% 85.53% +0.39%
==========================================
Files 60 60
Lines 18929 19020 +91
==========================================
+ Hits 16116 16268 +152
+ Misses 2813 2752 -61 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds a new preview global parameter, QUIC_PARAM_GLOBAL_WORKER_STATISTICS, to expose per-worker timing statistics (active vs wall time) from the library’s worker pool via MsQuic->GetParam(NULL, ...). This fits into MsQuic’s diagnostics/observability surface alongside existing global perf counters and other global params.
Changes:
- Introduces new preview public structs
QUIC_WORKER_STATISTICSandQUIC_WORKER_STATISTICS_LIST, plus the new global param ID inmsquic.h. - Tracks worker active time at idle points (event-queue waits and scheduler yields) and adds a platform worker-pool API to snapshot per-worker stats.
- Adds parameter-validation tests (user + kernel test harness registration) for querying/validating the new statistics.
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/MsQuicTests.h | Adds new test declarations for worker statistics. |
| src/test/lib/ApiTest.cpp | Implements new tests to query and validate worker statistics snapshots. |
| src/test/bin/winkernel/control.cpp | Registers the new tests in the kernel-mode test driver dispatcher. |
| src/test/bin/quic_gtest.cpp | Adds new gtest cases to invoke the new worker statistics tests. |
| src/platform/platform_worker.c | Adds per-worker timing accumulation and implements CxPlatWorkerPoolGetStatistics. |
| src/inc/quic_platform.h | Declares CxPlatWorkerPoolGetStatistics for worker-pool snapshots. |
| src/inc/msquic.h | Adds preview structs and the new QUIC_PARAM_GLOBAL_WORKER_STATISTICS global param constant. |
| src/core/library.c | Implements QUIC_PARAM_GLOBAL_WORKER_STATISTICS in global param dispatch. |
a9694eb to
cbd435b
Compare
96de8bf to
326c9b0
Compare
Adds a preview global parameter to retrieve per-worker active and wall time statistics. Active time is tracked via timestamps in the platform worker loop. Kernel mode returns QUIC_STATUS_NOT_SUPPORTED. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
326c9b0 to
c4af934
Compare
…, UniquePtrArray Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Protect QUIC_PARAM_GLOBAL_WORKER_STATISTICS from concurrent teardown of the platform worker pool. The query now captures MsQuicLib.WorkerPool and takes a rundown reference (CXPLAT_WORKER_POOL_REF_STATS) under the library lock, releasing the lock before doing work on the referenced pool. To keep the pointer from being freed under a reader, MsQuicExecutionCreate and MsQuicExecutionDelete now mutate MsQuicLib.WorkerPool under the same lock and never leave it dangling (null under the lock, delete/create outside). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| { | ||
| if (Worker->Stats.ActiveStartTimeUs != 0) { | ||
| Worker->Stats.CumulativeActiveTimeUs += | ||
| CxPlatTimeDiff64(Worker->Stats.ActiveStartTimeUs, CxPlatTimeUs64()); |
There was a problem hiding this comment.
Generally, looks fine, but you need to be careful with all the CxPlatTimeUs64() calls, as they are quite expensive. Especially since I believe the processing call (once you dequeue an event) almost immediately calls it again.
There was a problem hiding this comment.
The timestamp in the worker loop is snapped again only after the events have been processed. I considered re-ordering the loop so that Worker->TimeNow can be used as the ActiveStartTimeUs, that might be worth it.
There was a problem hiding this comment.
Looking at it again, we currently snap time:
- right before running execution context (prior to this PR)
- right before processing events (added by this PR)
Both are potentially time consuming, so it seems reasonable to me to snap the time again - at least because the ECs seems to need a pretty fresh timestamp (see the comment about solving a potential race, it isn't clear which race this is but I suspect it has something to do with timers being already expired).
Description
Add per-worker statistics accessible via
MsQuic->GetParam(NULL, QUIC_PARAM_GLOBAL_WORKER_STATISTICS, ...):QUIC_WORKER_STATISTICSstruct:IdealProcessor,CumulativeActiveTimeUs,CumulativeWallTimeUsper CXPLAT workerQUIC_WORKER_STATISTICS_LISTheader withWorkerCountandWorkerStatsSizefor forward-compatible stride-based iterationIt tracks the amount of time a worker is active or idle. Note that a worker is considered active when it is preempted while processing a job: this isn't a measure of how much CPU time used by the worker and rather a measure of its work load.
Preview-guarded in public header (
QUIC_API_ENABLE_PREVIEW_FEATURES).Testing
Test added
Documentation
TBD