Add `run_scoped` method to run closures and conditionally perform partial resets
This PR adds a "CheckPoint" struct that can be used to save a position for a Bump that can be later reset to, allowing for partial resets of the allocator.
I have a use case where I build a collection from many complex structs that use a bump allocator, however these structs must pass a filter before being added to the collection, In cases where very few events make it through the filter I found that the bump allocator would grow to be massive due to leaked allocations from rejected structs. After testing using a second bump allocator as scratch space that resolved the memory leak issue however caused a ~20% performance hit due to the extra memory copies, this PR solves the problem by taking a checkpoint before building the complex struct, then performing a partial reset if the struct is rejected.
Not sure if this kind of conditional logic is used elsewhere but figured I'd make a PR anyway in case it's useful, happy to update docs etc if needed.
Thanks for the feedback!
I had a read through #105 and came up with run_scoped as a minimal adaptation of my current code.
I made checkpoint and the associated methods private, then added a run_scoped method that provides temporary access to the allocator. If the function returns Some(T) then the result can persist on the Bump, but if the function returns None then the checkpoint is restored and the position of the allocator is reset.
The API could definitely be polished up a bit, maybe adding a Result variant as well so it's not just Option but I think it addresses the potential issues with the borrow checker.
As for functionality, the run_scoped method was sufficient for my use case at least - you can see the update for the changed api here: https://codeberg.org/Kryesh/crystalline/commit/da364cd0781bfbb88632ce1ec8c4e1a60a43737f?style=split&whitespace=show-all
My app probably shouldn't be used as a reference however since I do bunch of casts to 'static
No problem,
I think that approach makes sense, sounds like the first 2 steps are pretty close to my original PR but with the reset function marked as unsafe - is that right?
If so I'll update the PR to match, I won't look at implementing Bump::with_scope though since that approach looks like it needs something more complex that just borrowing &self like I've got here.
Thanks for the tip on run_scoped as well! will definitely use that
I think that approach makes sense, sounds like the first 2 steps are pretty close to my original PR but with the reset function marked as unsafe - is that right?
Yes, and with the reset method taking &self instead of &mut self, so that it can be a proper unsafe building block for creating custom scoped behaviors like your desired maybe-reset-maybe-not.
Have updated to address some of the feedback, still need to add tests though - not familiar with quickcheck so will need to figure that out. Probably won't have time to look at it until the weekend
Have updated to address some of the feedback, still need to add tests though - not familiar with quickcheck so will need to figure that out. Probably won't have time to look at it until the weekend
Let me know if you have any questions about it or run into any roadblocks, happy to help out.