Conversation
3c2a748 to
1842e27
Compare
Codecov Report
@@ Coverage Diff @@
## dev #1118 +/- ##
==========================================
+ Coverage 55.92% 55.95% +0.02%
==========================================
Files 136 136
Lines 20065 20075 +10
Branches 4768 4770 +2
==========================================
+ Hits 11221 11232 +11
+ Misses 6003 5996 -7
- Partials 2841 2847 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
robgjansen
left a comment
There was a problem hiding this comment.
Easy-peasy. I just have some minor suggested changes. Thanks!
src/main/core/main.c
Outdated
| OPTION_EXPERIMENTAL_ENTRY( | ||
| "set-sched-fifo", 0, 0, G_OPTION_ARG_INT, &_setSchedFifo, | ||
| "Use the SCHED_FIFO scheduler. Requires CAP_SYS_NICE. See sched(7), capabilities(7) [0]", | ||
| "[0|1]") |
There was a problem hiding this comment.
I think we want this to be a boolean argument? Here is how we specify other boolean args (from options.c):
{"version", 'v', 0, G_OPTION_ARG_NONE, &(options->printSoftwareVersion),
"Print software version and exit", NULL},
{"pin-cpus", 'z', 0, G_OPTION_ARG_NONE, &(options->pinCPUs),
"Use experimental CPU pinning", NULL},
So I think this would just become:
OPTION_EXPERIMENTAL_ENTRY(
"set-sched-fifo", 0, 0, G_OPTION_ARG_NONE, &_setSchedFifo,
"Use the SCHED_FIFO scheduler. Requires CAP_SYS_NICE. See sched(7), capabilities(7)",
NULL)
There was a problem hiding this comment.
Ah I see. I'd previously looked for but couldn't find a way to suppoert e.g. --set-sched-fifo=true and --set-sched-fifo=false. It looks like if we wanted to add a way to explicitly disable it we could add another option --no-set-sched-fifo using the option G_OPTION_FLAG_REVERSE
src/main/core/main.c
Outdated
| fprintf(stderr, "Could not set SCHED_FIFO!\n"); | ||
| return -1; | ||
| } else { | ||
| fprintf(stderr, "In mode SCHED_FIFO.\n"); |
There was a problem hiding this comment.
Log this with our logger, probably at message level. How about:
message("Successfully set real-time scheduler mode to SCHED_FIFO");
src/main/core/main.c
Outdated
| int rc = sched_setscheduler(0, SCHED_FIFO, ¶m); | ||
|
|
||
| if (rc != 0) { | ||
| fprintf(stderr, "Could not set SCHED_FIFO!\n"); |
There was a problem hiding this comment.
The logger has already been set up at this point, so can we log this with our logger? How about:
error("Could not set real-time scheduler mode to SCHED_FIFO!")
7796b39 to
7acb2ef
Compare
Some initial analysis at https://github.com/sporksmith/phantom-paper/blob/realtime-experiment/experiments/realtime/analysis.ipynb
More analysis in progress; looks like cpu-pinning+realtime is a significant win in preload mode.
The ptrace modes are currently broken with the realtime scheduler.
Fixes #964