From 482189843220d4bd21ab4fdb737c20ee57580769 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 14 Jan 2026 19:16:09 -0800 Subject: [PATCH] std.Io.File.MemoryMap API tuning - remove file_size parameter from MemoryMap.write - remove requirement for mapping length to be aligned - align allocated fallback memory - add unit test for std.Io.Threaded.disable_memory_mapping = true - add unit test for MemoryMap.setLength --- lib/std/Io.zig | 2 +- lib/std/Io/File.zig | 6 +-- lib/std/Io/File/MemoryMap.zig | 29 ++++++----- lib/std/Io/Threaded.zig | 98 ++++++++++++++++++----------------- lib/std/Io/Threaded/test.zig | 57 ++++++++++++++++++++ lib/std/Io/test.zig | 27 +++++++--- 6 files changed, 145 insertions(+), 74 deletions(-) diff --git a/lib/std/Io.zig b/lib/std/Io.zig index b1af9f150c8779e1c0b42416bc9390ca90c5a27f..1da10a027fa172a208228600d7558378509d2094 100644 --- a/lib/std/Io.zig +++ b/lib/std/Io.zig @@ -658,7 +658,7 @@ pub const VTable = struct { fileMemoryMapDestroy: *const fn (?*anyopaque, *File.MemoryMap) void, fileMemoryMapSetLength: *const fn (?*anyopaque, *File.MemoryMap, n: usize) File.MemoryMap.SetLengthError!void, fileMemoryMapRead: *const fn (?*anyopaque, *File.MemoryMap) File.ReadPositionalError!void, - fileMemoryMapWrite: *const fn (?*anyopaque, *File.MemoryMap, file_size: u64) File.WritePositionalError!void, + fileMemoryMapWrite: *const fn (?*anyopaque, *File.MemoryMap) File.WritePositionalError!void, processExecutableOpen: *const fn (?*anyopaque, File.OpenFlags) std.process.OpenExecutableError!File, processExecutablePath: *const fn (?*anyopaque, buffer: []u8) std.process.ExecutablePathError!usize, diff --git a/lib/std/Io/File.zig b/lib/std/Io/File.zig index dda3f9fe8e809a8fc392a5570e23c3a991b34256..e7114653517573be5a27d0573f3e41c06fb0c2ac 100644 --- a/lib/std/Io/File.zig +++ b/lib/std/Io/File.zig @@ -68,11 +68,7 @@ pub const Stat = struct { ctime: Io.Timestamp, /// Smallest chunk length in bytes appropriate for optimal I/O. This will /// be set to `1` for operating systems or file systems that do not - /// recognize this concept. Not always a power of two. When creating a - /// `MemoryMap`, the mapping length must be a multiple of this value. - /// - /// On Windows, this is whichever is larger: PageSize or - /// AllocationGranularity. + /// recognize this concept. Not always a power of two. block_size: BlockSize, }; diff --git a/lib/std/Io/File/MemoryMap.zig b/lib/std/Io/File/MemoryMap.zig index 3751a0c76db4486e914d418387fb0541e7089e5e..1c3c6d5fa2a32314483a4ed3c66f1750ff8240f5 100644 --- a/lib/std/Io/File/MemoryMap.zig +++ b/lib/std/Io/File/MemoryMap.zig @@ -13,8 +13,8 @@ file: File, /// Byte index inside `file` where `memory` starts. Page-aligned. offset: u64, /// Memory that may or may not remain consistent with file contents. Use `read` -/// and `write` to ensure synchronization points. No minimum alignment on the -/// pointer is guaranteed, but the length is page-aligned. +/// and `write` to ensure synchronization points. Pointer is page-aligned but +/// length is not. memory: []u8, /// Tells whether it is memory-mapped or file operations. On Windows this also /// has a section handle. @@ -37,11 +37,13 @@ pub const CreateError = error{ } || Allocator.Error || File.ReadPositionalError; pub const CreateOptions = struct { - /// Size of the mapping, in bytes. If this is longer than the file size, it - /// will be filled with zeroes. + /// Size of the mapping, in bytes. If this is longer than the file size, + /// `memory` beyond the file end will be filled with zeroes and it is + /// unspecified whether, after calling `write`, the file length will be + /// set to `len` or remain unchanged. /// - /// Asserted to be a multiple of page size which can be obtained via - /// `std.heap.pageSize`. + /// This value has no minimum alignment requirement, but may gain + /// efficiency benefits from being a multiple of `File.Stat.block_size`. len: usize, /// When this has read set to false, bytes that are not modified before a /// sync may have the original file contents, or may be set to zero. @@ -81,10 +83,9 @@ pub fn setLength( mm: *MemoryMap, io: Io, /// New size of the mapping, in bytes. If this is longer than the file - /// size, it will be filled with zeroes. Asserted to be a multiple of page - /// size which can be obtained with `std.heap.pageSize`. + /// size, it will be filled with zeroes. No alignment requirement. new_length: usize, -) File.SetLengthError!void { +) SetLengthError!void { return io.vtable.fileMemoryMapSetLength(io.userdata, mm, new_length); } @@ -95,9 +96,9 @@ pub fn read(mm: *MemoryMap, io: Io) File.ReadPositionalError!void { /// Synchronizes the contents of `memory` to `file`. /// -/// Size of the mapping may be longer than the file size, so the `file_size` -/// argument is used to avoid writing too many bytes. If `file_size` is not -/// handy, use `File.length` to get it. -pub fn write(mm: *MemoryMap, io: Io, file_size: u64) File.WritePositionalError!void { - return io.vtable.fileMemoryMapWrite(io.userdata, mm, file_size); +/// If `memory.len` is greater than file size, the bytes beyond the end of the +/// file may be dropped, or they may be written, extending the size of the +/// file. +pub fn write(mm: *MemoryMap, io: Io) File.WritePositionalError!void { + return io.vtable.fileMemoryMapWrite(io.userdata, mm); } diff --git a/lib/std/Io/Threaded.zig b/lib/std/Io/Threaded.zig index e4fd954707097a9437d00eea2f9ff8832d86128b..bbd375e21f68fe31bb547ff7bdc08095f11d5f57 100644 --- a/lib/std/Io/Threaded.zig +++ b/lib/std/Io/Threaded.zig @@ -16172,8 +16172,6 @@ fn fileMemoryMapCreate( const offset = options.offset; const len = options.len; - assert(std.mem.isAligned(len, std.heap.page_size_min)); - if (!t.disable_memory_mapping) { if (createFileMap(file, options.protection, offset, options.populate, len)) |result| { return result; @@ -16187,12 +16185,13 @@ fn fileMemoryMapCreate( } const gpa = t.allocator; + const page_size = std.heap.pageSize(); + const alignment: Alignment = .fromByteUnits(page_size); const memory = m: { - const ptr = gpa.rawAlloc(len, .@"1", @returnAddress()) orelse - return error.OutOfMemory; + const ptr = gpa.rawAlloc(len, alignment, @returnAddress()) orelse return error.OutOfMemory; break :m ptr[0..len]; }; - errdefer gpa.rawFree(memory, .@"1", @returnAddress()); + errdefer gpa.rawFree(memory, alignment, @returnAddress()); if (!options.undefined_contents) try mmSyncRead(file, memory, offset); @@ -16363,7 +16362,7 @@ fn fileMemoryMapDestroy(userdata: ?*anyopaque, mm: *File.MemoryMap) void { } } else { const gpa = t.allocator; - gpa.rawFree(memory, .@"1", @returnAddress()); + gpa.rawFree(memory, .fromByteUnits(std.heap.pageSize()), @returnAddress()); } mm.* = undefined; } @@ -16374,46 +16373,54 @@ fn fileMemoryMapSetLength( new_len: usize, ) File.MemoryMap.SetLengthError!void { const t: *Threaded = @ptrCast(@alignCast(userdata)); - assert(std.mem.isAligned(new_len, std.heap.page_size_min)); - if (mm.section) |section| switch (native_os) { - .windows => { - _ = section; - @panic("TODO"); - }, - .wasi => unreachable, - else => { - const flags: posix.MREMAP = .{ .MAYMOVE = true }; - const addr_hint: ?[*]const u8 = null; - const new_memory = while (true) { - const syscall: Syscall = try .start(); - const rc = posix.system.mremap(mm.memory.ptr, mm.memory.len, new_len, flags, addr_hint); - syscall.finish(); - const err: posix.E = if (builtin.link_libc) e: { - if (rc != std.c.MAP_FAILED) break @as([*]u8, @ptrCast(@alignCast(rc)))[0..new_len]; - break :e @enumFromInt(posix.system._errno().*); - } else e: { - const err = posix.errno(rc); - if (err == .SUCCESS) break @as([*]u8, @ptrFromInt(rc))[0..new_len]; - break :e err; + const page_size = std.heap.pageSize(); + const alignment: Alignment = .fromByteUnits(page_size); + + if (mm.section) |section| { + if (alignment.forward(new_len) == alignment.forward(mm.memory.len)) { + mm.memory.len = new_len; + return; + } + switch (native_os) { + .windows => { + _ = section; + @panic("TODO"); + }, + .wasi => unreachable, + else => { + const flags: posix.MREMAP = .{ .MAYMOVE = true }; + const addr_hint: ?[*]const u8 = null; + const new_memory = while (true) { + const syscall: Syscall = try .start(); + const rc = posix.system.mremap(mm.memory.ptr, mm.memory.len, new_len, flags, addr_hint); + syscall.finish(); + const err: posix.E = if (builtin.link_libc) e: { + if (rc != std.c.MAP_FAILED) break @as([*]u8, @ptrCast(@alignCast(rc)))[0..new_len]; + break :e @enumFromInt(posix.system._errno().*); + } else e: { + const err = posix.errno(rc); + if (err == .SUCCESS) break @as([*]u8, @ptrFromInt(rc))[0..new_len]; + break :e err; + }; + switch (err) { + .SUCCESS => unreachable, + .INTR => continue, + .AGAIN => return error.LockedMemoryLimitExceeded, + .NOMEM => return error.OutOfMemory, + .INVAL => return errnoBug(err), + .FAULT => return errnoBug(err), + else => return posix.unexpectedErrno(err), + } }; - switch (err) { - .SUCCESS => unreachable, - .INTR => continue, - .AGAIN => return error.LockedMemoryLimitExceeded, - .NOMEM => return error.OutOfMemory, - .INVAL => return errnoBug(err), - .FAULT => return errnoBug(err), - else => return posix.unexpectedErrno(err), - } - }; - mm.memory = new_memory; - }, + mm.memory = new_memory; + }, + } } else { const gpa = t.allocator; - if (gpa.rawRemap(mm.memory, .@"1", new_len, @returnAddress())) |new_ptr| { + if (gpa.rawRemap(mm.memory, alignment, new_len, @returnAddress())) |new_ptr| { mm.memory = new_ptr[0..new_len]; } else { - const new_ptr = gpa.rawAlloc(new_len, .@"1", @returnAddress()) orelse + const new_ptr = gpa.rawAlloc(new_len, alignment, @returnAddress()) orelse return error.OutOfMemory; const copy_len = @min(new_len, mm.memory.len); @memcpy(new_ptr[0..copy_len], mm.memory[0..copy_len]); @@ -16429,16 +16436,11 @@ fn fileMemoryMapRead(userdata: ?*anyopaque, mm: *File.MemoryMap) File.ReadPositi return mmSyncRead(mm.file, mm.memory, mm.offset); } -fn fileMemoryMapWrite( - userdata: ?*anyopaque, - mm: *File.MemoryMap, - file_size: u64, -) File.WritePositionalError!void { +fn fileMemoryMapWrite(userdata: ?*anyopaque, mm: *File.MemoryMap) File.WritePositionalError!void { const t: *Threaded = @ptrCast(@alignCast(userdata)); _ = t; if (mm.section != null) return; - const offset = mm.offset; - return mmSyncWrite(mm.file, mm.memory[0..@intCast(file_size - offset)], offset); + return mmSyncWrite(mm.file, mm.memory, mm.offset); } fn mmSyncRead(file: File, memory: []u8, offset: u64) File.ReadPositionalError!void { diff --git a/lib/std/Io/Threaded/test.zig b/lib/std/Io/Threaded/test.zig index 34aa4a8c689d67ceef1d1ca8e985b5487da9e2f1..f90ab6e745b5431aa438d03bad223e9cddeb76ec 100644 --- a/lib/std/Io/Threaded/test.zig +++ b/lib/std/Io/Threaded/test.zig @@ -204,3 +204,60 @@ test "cancel blocked read from pipe" { try io.sleep(.fromMilliseconds(10), .awake); try future.cancel(io); } + +test "memory mapping fallback" { + var threaded: std.Io.Threaded = .init(std.testing.allocator, .{ + .argv0 = .empty, + .environ = .empty, + .disable_memory_mapping = true, + }); + defer threaded.deinit(); + const io = threaded.io(); + + var tmp = testing.tmpDir(.{}); + defer tmp.cleanup(); + + try tmp.dir.writeFile(io, .{ + .sub_path = "blah.txt", + .data = "this is my data123", + }); + + { + var file = try tmp.dir.openFile(io, "blah.txt", .{ .mode = .read_write }); + defer file.close(io); + + // The `Io.File.MemoryMap` API does not specify what happens if we supply a + // length greater than file size, but this is testing specifically std.Io.Threaded + // with disable_memory_mapping = true. + var mm = try file.createMemoryMap(io, .{ .len = "this is my data123".len + 3 }); + defer mm.destroy(io); + + try testing.expectEqualStrings("this is my data123\x00\x00\x00", mm.memory); + mm.memory[4] = '9'; + mm.memory[7] = '9'; + + try mm.write(io); + } + + var buffer: [100]u8 = undefined; + const updated_contents = try tmp.dir.readFile(io, "blah.txt", &buffer); + try testing.expectEqualStrings("this9is9my data123\x00\x00\x00", updated_contents); + + { + var file = try tmp.dir.openFile(io, "blah.txt", .{ .mode = .read_only }); + defer file.close(io); + + var mm = try file.createMemoryMap(io, .{ + .len = "this9is9my".len, + .protection = .{ .read = true }, + }); + defer mm.destroy(io); + + try testing.expectEqualStrings("this9is9my", mm.memory); + + try mm.setLength(io, "this9is9my data123".len); + try mm.read(io); + + try testing.expectEqualStrings("this9is9my data123", mm.memory); + } +} diff --git a/lib/std/Io/test.zig b/lib/std/Io/test.zig index 43237fcb6f84c1eede28dc62b968eb0ec6d7b1dd..1cfcd7a22193eb842f7f06233cffddc6ae8baaa0 100644 --- a/lib/std/Io/test.zig +++ b/lib/std/Io/test.zig @@ -608,20 +608,35 @@ test "memory mapping" { var file = try tmp.dir.openFile(io, "blah.txt", .{ .mode = .read_write }); defer file.close(io); - const stat = try file.stat(io); - const aligned_len = std.mem.alignForward(usize, @intCast(stat.size), std.heap.pageSize()); - - var mm = try file.createMemoryMap(io, .{ .len = aligned_len }); + var mm = try file.createMemoryMap(io, .{ .len = "this is my data123".len }); defer mm.destroy(io); - try expectEqualStrings("this is my data123", std.mem.sliceTo(mm.memory, 0)); + try expectEqualStrings("this is my data123", mm.memory); mm.memory[4] = '9'; mm.memory[7] = '9'; - try mm.write(io, stat.size); + try mm.write(io); } var buffer: [100]u8 = undefined; const updated_contents = try tmp.dir.readFile(io, "blah.txt", &buffer); try expectEqualStrings("this9is9my data123", updated_contents); + + { + var file = try tmp.dir.openFile(io, "blah.txt", .{ .mode = .read_only }); + defer file.close(io); + + var mm = try file.createMemoryMap(io, .{ + .len = "this9is9my".len, + .protection = .{ .read = true }, + }); + defer mm.destroy(io); + + try expectEqualStrings("this9is9my", mm.memory); + + try mm.setLength(io, "this9is9my data123".len); + try mm.read(io); + + try expectEqualStrings("this9is9my data123", mm.memory); + } } -- 2.54.0