Support Thread Affinity for Execution Config #4614
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4614 +/- ##
==========================================
- Coverage 87.27% 86.79% -0.48%
==========================================
Files 56 56
Lines 17354 17357 +3
==========================================
- Hits 15145 15065 -80
- Misses 2209 2292 +83 ☔ View full report in Codecov by Sentry. |
| @@ -272,6 +272,7 @@ typedef enum QUIC_EXECUTION_CONFIG_FLAGS { | |||
| QUIC_EXECUTION_CONFIG_FLAG_XDP = 0x0004, | |||
| QUIC_EXECUTION_CONFIG_FLAG_NO_IDEAL_PROC = 0x0008, | |||
| QUIC_EXECUTION_CONFIG_FLAG_HIGH_PRIORITY = 0x0010, | |||
| QUIC_EXECUTION_CONFIG_FLAG_AFFINITIZE = 0x0020, | |||
There was a problem hiding this comment.
IMO, this should indicate what the threads are being affinitized to - NUMA node, single RSS CPU, something else?
There was a problem hiding this comment.
I'd prefer to just leverage documentation for what, which is currently being deferred since these are all prototype features. I just want to get this in so our partners can experiment. If it doesn't actually help, it might ultimately be removed.
There was a problem hiding this comment.
Generally speaking, I think even experimental features should be designed as if they will be retained, because it is difficult to remove/rename existing dependencies. That said, I'm ok with unblocking folks here.
| ThreadFlags |= CXPLAT_THREAD_FLAG_HIGH_PRIORITY; | ||
| } | ||
| if (MsQuicLib.ExecutionConfig->Flags & QUIC_EXECUTION_CONFIG_FLAG_AFFINITIZE) { | ||
| ThreadFlags |= CXPLAT_THREAD_FLAG_SET_AFFINITIZE; |
There was a problem hiding this comment.
Looks like just adding flag? where is the actual logic?
There was a problem hiding this comment.
The logic was already implemented elsewhere. This is just a new config flag.
Description
Adds another execution config knob for setting affinitizing threads to their CPU.
Testing
CI/CD
Documentation
N/A