rpmalloc icon indicating copy to clipboard operation
rpmalloc copied to clipboard

Some usage annoyances while compiling & using rpmalloc on Sony's consoles

Open n00bmind opened this issue 1 year ago • 1 comments

Hi! While integrating rpmalloc in a project that's compiled for Sony's consoles, I found some issues that even though minor, could improve the user experience if fixed. So I thought I would drop you a line and see what you think about them:

  1. The first issue is that I wasted a good couple days of weird crashes due to not reading the comments / documentation about the requirement for all returned OS blocks to be aligned to the span size. Obviously this is on me for not RTFM, but since this is (as I later learned) such a fundamental detail to the inner workings of the allocator, I would have expected for an assert to trigger somewhere, specially since I run with both ENABLE_VALIDATE_ARGS & ENABLE_ASSERTS always on on debug builds.

  2. When I finally figured out what was going on, I resorted to doing the exact same computation you do in _memory_map_os to manually align the blocks returned from the platform. However, the computation there uses the configured allocation granularity to figure out whether it's even necessary to add any padding or not. But, alas, there's no access to configured granularity anywhere that I could find. This also strikes me as a bit weird since there may be platforms where this needs to be specified in the config anyway? (Windows is a good example where this is different from the page size, if a Windows platform wanted to rely on the config interface alone, it would have no way of specifying this to rpmalloc_initialize). In my case I resorted to adding this additional field to the config struct.

One last thing, just to point out that the comment about the offset parameter returned from the memory_map function callback seems to be wrong / outdated. It's specified that this must be <= UINT16_MAX, but as I could see in the source code, this seems to be stored in either a u32 or a size_t field, so this requirement seems unnecessary. (I didn't check if this has been recently corrected, my version of the source code is easily a couple years old).

Anyway, all pretty minor stuff like I said.. thanks for the great work & thought you've obviously put into this project!

n00bmind avatar Aug 12 '22 15:08 n00bmind

Valid comments, I will fix these

mjansson avatar Aug 15 '22 06:08 mjansson