Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add repeated upsizing tests #129

Merged
merged 4 commits into from
Dec 21, 2024
Merged

Add repeated upsizing tests #129

merged 4 commits into from
Dec 21, 2024

Conversation

spaskalev
Copy link
Owner

Issue #128 reported a potential problem with repeated up-sizing the allocator. Add tests for this in both the normal and embedded modes. Instead of running the check invariant function (which also works but is a lot more slower) use the fragmentation metric. The basis of reasoning here is that if the resize does not alter the allocated slots in any way then the fragmentation metric should not change either.

Also fixed some formatting nits while at it.

@spaskalev
Copy link
Owner Author

@janedoe-lab please take a look, using this approach might help you in your case.

@spaskalev
Copy link
Owner Author

The 32-bit failure is interesting, it looks like there really is a bug there.

@spaskalev spaskalev force-pushed the spaskalev/upsizing-tests branch from 425ffb4 to 1a72050 Compare December 21, 2024 16:56
@spaskalev spaskalev merged commit f9c8dab into main Dec 21, 2024
6 checks passed
@spaskalev spaskalev deleted the spaskalev/upsizing-tests branch December 21, 2024 17:04
@janedoe-lab
Copy link

So I found some bugs in my codebase. But it was quite a massive overhaul. I can't replicate the bug from #128 anymore, but can't quite pinpoint which change did it. So right now I am 99.99% sure it was fully due to bugs in my codebase.

@spaskalev
Copy link
Owner Author

Glad to hear it. I did spend a few hours and the difference between 64 and 32 bit were expected so the allocator seems to be working fine in this scenario (16MB growing to 512MB). I documented that allocations are expected to remain when calling resize.

Note - resize is not optimized at all and it is slow, especially for large arenas with small alignment. You may want to pre-allocate a larger chunk if memory permits.

@janedoe-lab
Copy link

Thank you so much for all the work. So far it is rock-solid!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants