Proposal: SDL_MemoryMapFile

[kieselsteini]

Hello there,

I've a small proposal for SDL3 here. There is the already well known SDL_LoadFile function which comes in very handy. But it would be also very handy, to have a similar function to effectively memory map a file. On most Unix systems this can be achieved with the mmap syscall and on Windows there is CreateFileMapping.

The signature of the function could look like:

const Uint8* SDL_MemoryMapFile(const char *filename, size_t *datasize)

Of course this will memory map the requested file as read only.

I can try to implement a first version of this feature, but I've no Windows box to test it.

[slouken]

You'd want an offset parameter as well, I believe, but this seems super useful!

[icculus]

Is it actually beneficial to mmap a file?

(Discounting mmap as a device interface on Unix and for loading shared libraries and stuff...I just mean regular file i/o.)

[nfries88]

mmap is quite useful if you have a single (or very few) frequently accessed files that fit entirely into the virtual address space easily but are still large enough that forcing the entire thing into physical memory all at once (eg by simply reading the entire file) is undesirable.

its use for general purpose I/O is generally discouraged, but for the right pattern it can be much more efficient than using I/O syscalls. overuse can tank system performance by TLB pollution, and when an access actually causes a page fault it will actually perform worse than just doing the I/O would have because of the overhead of maintaining page mappings (eg using mmap will tend to outperform read() for small sequential accesses over a large file but not for large or random accesses)

[kieselsteini]

As @nfries88 explained, this can be very helpful when you have a few "large" asset archives which you can simply map to memory and then just directly access the data. The file will not be loaded at once to memory, but only the portions you access and the OS will also unload/discard allocated memory pages automatically for you.

A good example of it is the Chocolate Doom Project. They use mmap/CreateFileMapping on platforms that support it to "open" the DOOM.WAD archive (see w_file_posix.c they have also one for Win32). Even if the 14MB of that file will fit easily to RAM on machines nowadays. That was actually also the projects which inspired me, to use a similar pattern and then also to propose it here.

[madebr]

I remember this llama.cpp change last year.
The commit documents 3 advantages.

[nfries88]

honestly having portable support for shared memory objects would be convenient for custom high-performance tracing as well, and while it would be less than ideal (bc it becomes subject to the filesystem cache and doesn't conform to normal system-specific practices) it would be simple to implement that in terms of this modified to enable write access.

[icculus]

(I still think this is a no from me, unless @slouken feels strongly otherwise.)

[slouken]

I'm a fan. Does someone want to create a PR for this?

[kieselsteini]

I made a first attempt: PR-10960.
It's my first try to contribute here ;)

[maia-s]

Should this return const volatile Uint8*, since the mapped memory can change if the underlying file changes?

[nfries88]

Should this return const volatile Uint8*, since the mapped memory can change if the underlying file changes?

volatile is insufficient to appropriately deal with this possibility. at best it just makes the introduced inconsistency appear faster.

[maia-s]

Yeah, it's unsafe either way (if the file shrinks, accessing the removed bytes can segfault), but wouldn't volatile be a little bit safer at least? Ig it depends on the intent of the user. If they expect changes, they want the changes to appear as soon as possible, but if they expect the file to be constant, they don't. But the lack of volatile won't help in that case.

[nfries88]

volatile is insufficient when anticipating those changes as well. You really need to use atomics or at least explicit CPU memory barriers if you're using a memory mapping for IPC or similar. volatile might be enough to make this almost safe on x86 but it hurts the performance of every other use case (and possibly even that use case as well)

[slouken]

You should never use volatile. It implies a level of safety that isn't really there. Always use explicit memory barriers and so forth for IPC and multi-threaded code.

[maia-s]

This has nothing to do with IPC or synchronising multi threaded code. Yes, you should never use volatile for that. That's not what's happening here.

What's happening here is that the OS can change the mapped bytes at any time without the compiler's knowledge. Volatile tells the compiler that that can happen.

At the very least this API needs a prominent warning that it's fundamentally unsafe to use.

[nfries88]

the OS can change the mapped bytes at any time without the compiler's knowledge

It doesn't do this willy-nilly. Regular file mappings contain the contents of the disk cache, the only way they change is when an I/O write (or write through a memory mapping) is performed on the file by another thread or process, or a page fault forces a read from disk. In the latter case volatile is totally unnecessary, in the prior case the IPC requirements still apply.

Volatile maybe makes more sense for device memory mappings, but that's not really what this proposal is about.

[maia-s]

Yes, that's the point. You have no control of what other processes do with the file you mmap. And if the file is shrunk, your app can crash and there's nothing you can do about it.

[nfries88]

volatile just doesn't help with that in any meaningful way.

[maia-s]

It doesn't help with the crashing, no. Nothing you can do about that, like I said.

For the rest it means you won't get a mix of data the compiler thinks it can cache and data from the file if it is changed. You can still get a mix of old and new file data, but afaik you won't get old data after you get new. The file can have been partially updated when you read it tho.

[nfries88]

and generally the only way to properly deal with that when it happens is to go back and start over your current I/O related task from scratch, and even catching it requires you build a bunch of extra logic into the program (and probably bake extra data into the file format) in order to be able to validate every I/O operation. if you add checksums to your packfile format and check against them after every read operation, volatile or not still doesn't really matter.

[ITotalJustice]

i made a linux version of sdl_mmap, it features almost everything mmap can do.
i added a wrapper for IOStream as well. i havent made it a pr because i haven't (and won't) made a windows/mac version. unless you'd accept a pr thats only linux for now 😄

https://gist.github.com/ITotalJustice/eb9c47d0fdbc34e275b2ca79460bf0e3

[icculus]

( Reminder in case this merges: add it to https://wiki.libsdl.org/SDL3/NewFeatures )