Setting value of item to undefined can cause performance degradation when
runtime safety is enabled. This allows user to opt-out when that
degradation is not acceptable
Resolves #35627
psznm/zig:memory-pool-unsafe-variants into master
Setting value of item to undefined can cause performance degradation when
runtime safety is enabled. This allows user to opt-out when that
degradation is not acceptable
Resolves #35627
d12c33ef78
172e2995ae
@ -126,6 +133,13 @@ pub fn Extra(comptime Item: type, comptime pool_options: Options) type {
/// Only pass items to `ptr` that were previously created with `create()` of the same memory pool!
pubfndestroy(pool:*Pool,ptr:ItemPtr)void{
ptr.*=undefined;
destroyUnsafe(pool,ptr);
this call would need to go before, and adding these methods feels like the wrong solution
a particularly useful piece that would help progress on the issue you opened is a script that exhibits the performance problem described so that other folks can reproduce it, and benchmark it, as well
@nektro wrote in #35628 (comment):
this call would need to go before, and adding these methods feels like the wrong solution
It is in the same order as it was before.
What other solution would you prefer? Options on the MemoryPool?
It is in the same order as it was before.
sorry about that, my misunderstanding
Correct me if I'm wrong, but on 0.16.0, compiling on ReleaseFast correctly optimizes ptr.* = undefined away. In ReleaseSafe, in the case of .create, the assembly stays around, meaning the issue would lay with optimization.
movabs rax, -6148914691236517206
mov rcx, qword ptr [rbp - 48]
mov qword ptr [rcx + 16], rax
@setRuntimeSafety would not be able to optimize it away, as currently it would not handle optimization. #978 would be one way to solve it, however, assuming that a future @optimizeFor inherits the traits of @setRuntimeSafety - making it lexically scoped, rather than dynamically scoped -it would mean that using it within the scope of create won't help.
@optimizeFor(.ReleaseFast);
memory_pool.create(...); // would still call `ptr.* = undefined;`
If @optimizeFor is to be implemented with dynamic scope, this case would be handled as is. I'm not sure if this is possible, as I have limited experience with the compiler.
However, if @optimizeFor stays the same, this concern should probably also be addressed in order calls that leverage the lorem.* = undefined; pattern (such as MultiArrayList.deinit, Zip's Diagnostics.deinit, PriorityQueue.deinit)
No due date set.
No dependencies set.
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?