Skip to content

Commit 40b4a3c

Browse files
committed
linux optimization: frtuncate() while mapped
This is probably bad: If the size of the mapped file changes after the call to mmap() as a result of some other operation on the mapped file, the effect of references to portions of the mapped region that correspond to added or removed portions of the file is unspecified. Alternatives: - Don't msync() when unmapping/remapping - 90% of the perf hit - Investigate mremap()ing the whole range - mmap() just the grown region like I already do for ResizableMappedMemory
1 parent 977ad2a commit 40b4a3c

File tree

2 files changed

+100
-34
lines changed

2 files changed

+100
-34
lines changed

include/decodeless/detail/mappedfile_linux.hpp

Lines changed: 18 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -201,46 +201,40 @@ class ResizableMappedFile {
201201
ResizableMappedFile(const ResizableMappedFile& other) = delete;
202202
ResizableMappedFile(ResizableMappedFile&& other) noexcept = default;
203203
ResizableMappedFile(const fs::path& path, size_t maxSize)
204-
: m_reserved(nullptr, maxSize, MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE, -1, 0)
205-
, m_file(path, O_CREAT | O_RDWR, 0666) {
206-
size_t size = throwIfAbove(m_file.size(), m_reserved.size());
207-
if (size)
208-
map(size);
209-
}
204+
: m_file(path, O_CREAT | O_RDWR, 0666)
205+
, m_size(throwIfAbove(m_file.size(), maxSize))
206+
// Map the entire reserved range (previously a separate MAP_PRIVATE
207+
// mapping was created). SIGBUS is raised on out of bounds access and
208+
// calling ftruncate() without remapping seems to just work. I'll take
209+
// the improved perf!
210+
, m_mapped(nullptr, maxSize, MAP_SHARED, m_file, 0) {}
210211
ResizableMappedFile& operator=(const ResizableMappedFile& other) = delete;
211-
void* data() const { return m_mapped ? m_mapped->address() : nullptr; }
212-
size_t size() const { return m_mapped ? m_mapped->size() : 0; }
213-
size_t capacity() const { return m_reserved.size(); }
212+
void* data() const { return m_size != 0 ? m_mapped.address() : nullptr; }
213+
size_t size() const { return m_size; }
214+
size_t capacity() const { return m_mapped.size(); }
214215
void resize(size_t size) {
215-
size = throwIfAbove(size, m_reserved.size());
216-
m_mapped.reset();
217-
m_file.truncate(size);
218-
if (size)
219-
map(size);
216+
m_file.truncate(throwIfAbove(size, capacity()));
217+
m_size = size;
220218
}
221219

222-
// Override default move assignment so m_reserved outlives m_mapped
220+
// Override default move assignment so m_file outlives m_mapped
221+
// TODO: fix this by having the mapping own the file descriptor
223222
ResizableMappedFile& operator=(ResizableMappedFile&& other) noexcept {
224223
m_mapped = std::move(other.m_mapped);
224+
m_size = other.m_size;
225225
m_file = std::move(other.m_file);
226-
m_reserved = std::move(other.m_reserved);
227226
return *this;
228227
}
229228

230229
private:
231-
void map(size_t size) {
232-
// TODO: if m_mapped shrinks, does m_reserved instead need to be
233-
// recreated to fill the gap?
234-
m_mapped.emplace(m_reserved.address(), size, MAP_FIXED | MAP_SHARED, m_file, 0);
235-
}
236230
static size_t throwIfAbove(size_t v, size_t limit) {
237231
if (v > limit)
238232
throw std::bad_alloc();
239233
return v;
240234
}
241-
detail::MemoryMap<PROT_NONE> m_reserved;
242-
FileDescriptor m_file;
243-
std::optional<detail::MemoryMapRW> m_mapped;
235+
FileDescriptor m_file;
236+
size_t m_size;
237+
detail::MemoryMapRW m_mapped;
244238
};
245239

246240
static_assert(std::is_move_constructible_v<ResizableMappedFile>);

test/src/mappedfile.cpp

Lines changed: 82 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ TEST_F(MappedFileFixture, FileHandle) {
3737
EXPECT_TRUE(file); // a bit pointless - would have thrown if not
3838
}
3939

40-
TEST_F(MappedFileFixture, Create) {
40+
TEST(MappedFile, Create) {
4141
fs::path tmpFile2 = fs::path{testing::TempDir()} / "test2.dat";
4242
EXPECT_FALSE(fs::exists(tmpFile2));
4343
if (fs::exists(tmpFile2))
@@ -64,7 +64,7 @@ TEST_F(MappedFileFixture, Create) {
6464
EXPECT_FALSE(fs::exists(tmpFile2));
6565
}
6666

67-
TEST_F(MappedFileFixture, Reserve) {
67+
TEST(MappedFile, Reserve) {
6868
fs::path tmpFile2 = fs::path{testing::TempDir()} / "test2.dat";
6969
{
7070
// Create a new file
@@ -125,7 +125,57 @@ TEST_F(MappedFileFixture, LinuxFileDescriptor) {
125125
EXPECT_NE(fd, -1);
126126
}
127127

128-
TEST_F(MappedFileFixture, LinuxCreate) {
128+
// Won't catch SIGBUS
129+
#if 0
130+
#include <setjmp.h>
131+
#include <signal.h>
132+
jmp_buf* g_sigbusJmpbufPtr = nullptr;
133+
TEST_F(MappedFileFixture, LinuxOvermapWrite) {
134+
EXPECT_EQ(fs::file_size(m_tmpFile), sizeof(int));
135+
size_t overmapSize = 1024 * 1024 * 1024;
136+
detail::FileDescriptor fd(m_tmpFile, O_RDWR);
137+
detail::MemoryMap<PROT_READ | PROT_WRITE> mapped(nullptr, overmapSize, MAP_SHARED, fd, 0);
138+
auto signalHandler = []([[maybe_unused]] int sig) {
139+
if (sig == SIGBUS)
140+
siglongjmp(*g_sigbusJmpbufPtr, 1);
141+
};
142+
jmp_buf sigbusJmpbuf;
143+
g_sigbusJmpbufPtr = &sigbusJmpbuf;
144+
if (sigsetjmp(sigbusJmpbuf, 1) == 0) {
145+
signal(SIGBUS, signalHandler);
146+
// Write way past the end of the file's region. This should raise SIGBUS
147+
std::span data(reinterpret_cast<uint8_t*>(mapped.address()), mapped.size());
148+
data.back() = 142;
149+
EXPECT_TRUE(false) << "Expected SIGBUS";
150+
}
151+
signal(SIGINT, SIG_DFL);
152+
g_sigbusJmpbufPtr = nullptr;
153+
EXPECT_EQ(fs::file_size(m_tmpFile), sizeof(int));
154+
}
155+
#endif
156+
157+
TEST_F(MappedFileFixture, LinuxOvermapResizeWrite) {
158+
size_t overmapSize = 1024 * 1024;
159+
EXPECT_EQ(fs::file_size(m_tmpFile), sizeof(int));
160+
{
161+
detail::FileDescriptor fd(m_tmpFile, O_RDWR);
162+
detail::MemoryMapRW mapped(nullptr, overmapSize, MAP_SHARED, fd, 0);
163+
fd.truncate(overmapSize);
164+
165+
std::span data(reinterpret_cast<uint8_t*>(mapped.address()), mapped.size());
166+
data.back() = 142;
167+
}
168+
EXPECT_EQ(fs::file_size(m_tmpFile), overmapSize);
169+
{
170+
std::ifstream ifile(m_tmpFile, std::ios::binary);
171+
uint8_t lastByte;
172+
ifile.seekg(overmapSize-sizeof(lastByte));
173+
ifile.read(reinterpret_cast<char*>(&lastByte), sizeof(lastByte));
174+
EXPECT_EQ(lastByte, 142);
175+
}
176+
}
177+
178+
TEST(MappedFile, LinuxCreate) {
129179
fs::path tmpFile2 = fs::path{testing::TempDir()} / "test2.dat";
130180
EXPECT_FALSE(fs::exists(tmpFile2));
131181
{
@@ -157,7 +207,7 @@ TEST_F(MappedFileFixture, LinuxReserve) {
157207
EXPECT_EQ(*reinterpret_cast<const int*>(mapped.address()), 42);
158208
}
159209

160-
TEST_F(MappedFileFixture, LinuxResize) {
210+
TEST(MappedFile, LinuxResize) {
161211
// Reserve some virtual address space
162212
detail::MemoryMap<PROT_NONE> reserved(nullptr, detail::pageSize() * 4,
163213
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
@@ -221,10 +271,6 @@ TEST_F(MappedFileFixture, LinuxResize) {
221271
EXPECT_FALSE(fs::exists(tmpFile2));
222272
}
223273

224-
// TODO:
225-
// - MAP_HUGETLB
226-
// - MAP_HUGE_2MB, MAP_HUGE_1GB
227-
228274
#endif
229275

230276
TEST(MappedMemory, ResizeMemory) {
@@ -405,8 +451,34 @@ TEST_F(MappedFileFixture, ResizableFileSize) {
405451
EXPECT_EQ(fs::file_size(m_tmpFile), lastSize);
406452
}
407453

408-
TEST_F(MappedFileFixture, Readme) {
409-
fs::path tmpFile2 = fs::path{testing::TempDir()} / "test2.dat";
454+
TEST(MappedFile, Empty) {
455+
fs::path tmpFile2 = fs::path{testing::TempDir()} / "test2.dat";
456+
EXPECT_FALSE(fs::exists(tmpFile2));
457+
{
458+
size_t maxSize = 4096;
459+
resizable_file file(tmpFile2, maxSize);
460+
EXPECT_TRUE(fs::exists(tmpFile2));
461+
EXPECT_EQ(file.size(), 0);
462+
}
463+
EXPECT_TRUE(fs::exists(tmpFile2));
464+
EXPECT_EQ(fs::file_size(tmpFile2), 0);
465+
fs::remove(tmpFile2);
466+
EXPECT_FALSE(fs::exists(tmpFile2));
467+
}
468+
469+
TEST_F(MappedFileFixture, ClearExisting) {
470+
EXPECT_EQ(fs::file_size(m_tmpFile), sizeof(int));
471+
{
472+
size_t maxSize = 4096;
473+
resizable_file file(m_tmpFile, maxSize);
474+
EXPECT_EQ(file.size(), sizeof(int));
475+
file.resize(0);
476+
}
477+
EXPECT_EQ(fs::file_size(m_tmpFile), 0);
478+
}
479+
480+
TEST(MappedFile, Readme) {
481+
fs::path tmpFile2 = fs::path{testing::TempDir()} / "test2.dat";
410482
{
411483
size_t maxSize = 4096;
412484
resizable_file file(tmpFile2, maxSize);

0 commit comments

Comments
 (0)