search

ReversingSpace / cpp-gamefilesystem

Game and general prototyping filesystem support.
Other
1 stars 2 forks source link

PlatformFile: Performance #9

Open awstanley opened 5 years ago

awstanley commented 5 years ago

At the moment the gfs::PlatformFile wraps storage::File. There's nothing particularly wrong with that, except that the view is opened, closed, and then flushed, after each read and write. To partially alleviate that, I exposed the underlying file using the API.

One solution to the problem may be to to maintain a sane view size of a predetermined size (defined in a header as a constant, probably). But it comes with the headache of maintaining the seek positions by hand against two cursor values (though in reality it doesn't, it would require adding get_file_offset() to the View object; I'm not opposed to this, it's just a question of whether it's worth it for general use). The alternative is modifying the view to get an absolute offset (file_offset + cursor internally, and then returned).

Another solution is to replace it with something simpler (like a stream), but that introduces similar (if not greater?) overheads in many cases.

Due to the way the system is configured the PlatformFile can be replaced fairly easily from Directory instances through the StorageServer. The question is whether or not it's a sane default for the purposes of indie games, modding, and tooling. Obviously I designed it in such a way that it should be (at least in the way I use it), but I know it's also not ideal for streaming from disk (though it could be if the read size were sufficient to pull the view and memcpy out).

awstanley commented 5 years ago

I'm preparing the Archive abstraction layer/work at the moment and I'm adding the following:

These aren't particularly significant, but as inline consts they should be lightweight enough, clean enough, and sane enough, to function as needed. Obviously they can be shifted out of inline status at some point, but for now I'd rather leave them there (as the function call cost is probably higher than it needs to be, and I don't want to rely on a compiler trying to determine inlining in a header, or the usual nightmare of dynamic/shared includes).

This addition should open the door to better performance for general purpose I/O via cursor work, but likely needs to be supplemented.

Notably this issue is a partial blocker for Archive work because archives need to come from somewhere. Requiring the system to work around (manually) isn't ideal, though it's probably not the end of the world either; archives and general pathing have to be done somewhere, and trying to force them all into the root system strikes me as a bad idea (at least presently).

I know for business app logic it would be a terrible idea to merge it into it, so I'm wondering if anyone has any detracting views. (Looking mostly at @Brand2 and @metasymphony for perspectives on "is it too much to ram it into the core".) The other alternative is an optional (header-only/inlined/templated?) Archive loader system that can do path lookups based on directories and auto-mount as needed.