Fix page sizes #1184
Fix page sizes #1184
Conversation
mprotect needs its slices page-aligned. Since the VirtualList range is contiguous, using a larger page size wouldn't necessarily reduce memory anyways. And since pageSize() will only get called when allocating, it is not particularly expensive.
src/utils/list.zig
Outdated
| @@ -474,10 +476,10 @@ fn releaseMemory(start: [*]align(pageSize) u8, len: usize) void { | |||
| /// A list that reserves a continuous region of virtual memory without actually committing its pages. | |||
| /// This allows it to grow without ever invalidating pointers. | |||
| pub fn VirtualList(T: type, maxSize: u32) type { | |||
| const maxSizeBytes = std.mem.alignForward(usize, @as(usize, maxSize)*@sizeOf(T), pageSize); | |||
| std.debug.assert(@sizeOf(T) <= pageSize); | |||
| const maxSizeBytes = std.mem.alignForward(usize, @as(usize, maxSize)*@sizeOf(T), pageSize()); | |||
There was a problem hiding this comment.
How does this even work? Aren't you calling pageSize in a comptime context here (because the function returns a type)?
There was a problem hiding this comment.
Lol. I knew this would come up but I wasn't allowed to put a comptime check into pageSize(). I'll just make maxSizeBytes runtime determined then.
There was a problem hiding this comment.
Wait, but what does it return if you call it at compile time???
There was a problem hiding this comment.
It errors with
compiler/zig/lib/std/atomic.zig:16:20: error: unable to evaluate comptime expression
return @atomicLoad(T, &self.raw, order);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~There was a problem hiding this comment.
Why didn't you see that while testing then?
There was a problem hiding this comment.
I mean this is basically the only additional bug that can happen from aarch64-linux.
There was a problem hiding this comment.
Although I did mess it up so I'll add it in.
There was a problem hiding this comment.
It seems that Zig tries to cross-compile when the target is specified, which of course we can't do without preparing libGL and libX11, and that is very out of scope for this PR.
There was a problem hiding this comment.
Yeah, definitely not in this PR.
We could compile for aarch64-windows then? That should fail in the same cases.
There was a problem hiding this comment.
I made an issue for it: #1185
Cubyz once switched to its own
pageSize, but since then the standard library was upgraded to better represent hardware and software variation. This PR allows for running on ARM Linux and MacOS.