Conversation
e80f3db to
09ab7c7
Compare
|
Amazing! How is the performance? libmpq is pretty fast and reuses the decompression buffer (per thread).
Some of the file-number stuff was intentional to avoid computing the number multiple times, though I'm not sure how important that micro-optimization really is. |
Yeah from what I saw it should make very little difference, I could look into reintroducing it, but it would probably make the interface a lot less clean |
7a5fe9e to
8842910
Compare
|
@glebm applied some optimizations to mpqfs (reuse compression buffer) and things should be ready for review now. |
I think it'd just require adding a MPQFS_API bool mpqfs_has_file_hash(mpqfs_archive_t *archive, uint32_t hash);
MPQFS_API SDL_IOStream *mpqfs_open_io_from_hash(mpqfs_archive_t *archive, uint32_t hash);
// etc... |
|
Alright I think I got the optimizations working |
|
I'm a little unsure about disabeling compression support on OG Xbox, but hopefully it won't need it for the base game. |
|
Can you please also update: DevilutionX/tools/make_src_dist.py Line 40 in 69e5632 |
|
Looking at the writer implementation in mpqfs, it appears to keep all of the archive contents in RAM, which can increase peak RAM usage (save files can be > 1 MiB). DevilutionX implementation is leaner -- it writes the files directly to the archive as they get added. |
|
Yeah mpqfs write implementation is currently just good enough to produce valid saves, but not good enough to be used. I was wondering if I should just strip it out but ideally I would prefer to move all MPQ support to mpqfs rather then having an internal implementation for writing like we currently do. I'll see about fixing compression on Xbox and making the write support ready to replace ours. Thanks for going over the code. |
|
Makes sense. I also think that it might be cleaner to remove the SDL stuff from mpqfs. This is because multiple SDL versions are supported, which might necessitate multiple mpqfs packages (mpqfs-sdl2, mpqfs-sdl3, mpqfs-no-sdl, etc), if mpqfs is ever used by projects other than DevilutionX. |
|
Yeah I did consider that might not be ideal if it was to be used by a save game viewer. I didn't realize it could also affect how packaging works. The intent was to make it's interface to us as simple as possible. |
CPU Results (
Memory Results
|
048c041 to
0f8eab5
Compare
|
… unexpected error code
mpqfs is a minimal MIT-licensed MPQ v1 reader/writer that replaces both libmpq and PKWare in a single dependency.
We use mpqfs from: https://github.com/diasurgical/mpqfs
Impact:
mpqfs_open_rwops()call.AssetRef,FindAsset, and all call sites.implode()/explode()API is replaced with simple buffer-to-buffer calls.P.s. despite the angry CI this does work on my machine ™️
I need some sleep before I can figure out the CI issue.
P.s.s. should I strip out the write feature or expand it and also replace that part of DevilutionX with mpqfs?
Update: Builds should work now, the intergration test hash is updated since the different compression algo produces a different but valid data stream.