You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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:
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.
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!
The text was updated successfully, but these errors were encountered:
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:
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.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 torpmalloc_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!
The text was updated successfully, but these errors were encountered: