Handle hot-path time syscalls in the shim#1224
Handle hot-path time syscalls in the shim#1224robgjansen merged 7 commits intoshadow:devfrom robgjansen:shmem-time
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #1224 +/- ##
==========================================
+ Coverage 55.47% 56.03% +0.56%
==========================================
Files 140 141 +1
Lines 20300 20443 +143
Branches 4910 4944 +34
==========================================
+ Hits 11261 11455 +194
+ Misses 6015 5931 -84
- Partials 3024 3057 +33
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
sporksmith
left a comment
There was a problem hiding this comment.
Looks great!
Might we want a command-line flag to disable this for benchmarking/debugging purposes?
src/shim/shim_syscall.c
Outdated
| #include "shim/shim_syscall.h" | ||
| #include "support/logger/logger.h" | ||
|
|
||
| #define SIMTIME_NANOS_PER_SEC 1000000000l |
There was a problem hiding this comment.
Why the ending 1's? And should we move these constants to a header shared between shadow and the shim to ensure they stay sync'd?
There was a problem hiding this comment.
It's an l (a lower case L). I think I refactored this from a previously defined constant to this #define, but yeah we should use an existing definition or create one (e.g. in definitions.h).
src/shim/shim_syscall.c
Outdated
| _cached_simulation_time.tv_nsec; | ||
| } | ||
|
|
||
| bool shim_syscall_is_supported(long syscall_num) { |
There was a problem hiding this comment.
Instead of trying to keep this in sync with which syscalls are supported in shim_syscall, could the caller just try calling shim_syscall and fallback if it returns false?
There was a problem hiding this comment.
Yep, that was one wrinkle I noticed but left in place. But I agree and I'll change it.
Shadow stores the latest sim time in memory shared with the shim. When time-related syscalls are executed, the shim side can read the latest simulation time directly from shared memory and avoid the more expensive inter-process syscall serviced by shadow.
Fixes #1141