Avoid mirror mmap failures on newer kernel versions #474
Conversation
r1viollet
force-pushed
the
r1viollet/fix_mirror_mmap
branch
from
254e710 to
aa1d877
Compare
Benchmark results for collatzParameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics. See unchanged results
|
Benchmark results for BadBoggleSolver_runParameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics. See unchanged results
|
r1viollet
force-pushed
the
r1viollet/fix_mirror_mmap
branch
from
9ecaf0c to
aa1d877
Compare
r1viollet
changed the title
Manual test for ring buffer wrap logics
| if (required_size == 0) { | ||
| return rb.wrap_copy; | ||
| } | ||
| size_t const aligned_size = align_up(required_size, kRingBufferAlignment); |
There was a problem hiding this comment.
No need to bother about alignment, ring buffer alignment (8) will always be satisfied by memory returned by malloc / new.
There was a problem hiding this comment.
true, though code should be valid regardless of kRingBufferAlignment's value
There was a problem hiding this comment.
Better to keep things simple, static assert that kRingBufferAlignment is less than 16 and use make_unique:
inline std::byte *RingBuffer::ensure_wrap_copy_buffer(size_t required_size) {
static_assert(kRingBufferAlignment <= 16, "kRingBufferAlignment must be <= 16");
if (this->buffer_capacity < required_size) {
this->buffer = std::make_unique<std::byte[]>(required_size);
this->buffer_capacity = required_size;
}
return this->buffer.get();
}
| auto defer_munmap = make_defer([&]() { perfdisown(region, size_of_buffer); }); | ||
|
|
||
| auto *ptr = static_cast<std::byte *>(region); | ||
| PerfMmapRegion perfown_sz(int fd, size_t size_of_buffer, |
There was a problem hiding this comment.
Duplicating the function into two versions would make the code simpler
There was a problem hiding this comment.
I'm not sure I see it considering how we catch the failures.
Make ensure_wrap_copy_buffer part of the class
What does this PR do?
handle the case where we can't perform the double mapping
Motivation
User reported that we do not load on newer kernels.
Additional Notes
Tested on recent kernel
Before
Now we fall back to a single mapping.
How to test the change?
I had to boot a more recent kernel version.