Summary: would it be useful to add an IdAllocator mode or subclass that ensures that recently-deleted IDs don't get reissued prematurely?
The CHROMIUM_gpu_fence implementation from http://crrev.com/680135 uses client-side allocated IDs, but for performance reasons the CreateClientGpuFenceCHROMIUM command issues an out-of-order gpu_control->CreateGpuFence, and this might arrive at the service before a still-pending deletion.
The implementation avoids ID collisions by issuing them in strictly increasing order, and prevents wraparound by using a CHECK() to intentionally crash when it exhausts the ID space. This is unlikely to be an issue in practice for current use cases, this crash would happen after >400 days of continuous use with 2 fences per frame at 60fps.
For context, here's a potential scenario when the IDs wrap around where the current approach would result in a duplicate:
GLuint id_1 = CreateGpuFenceCHROMIUM();
// ... keep this fence around for a long time, ID counter wraps
DestroyGpuFenceCHROMIUM(id_1);
// Frees ID locally, server deletion is still pending
GLuint id_2 = CreateClientGpuFenceCHROMIUM(source);
// Allocates ID locally, reuses id_1 due to wrapping
gpu_control->CreateGpuFence(id_2, source);
// Arrives at server before the old GpuFence was deleted and fails.
It would be more elegant if the ID allocator could avoid the problematic recycling case more directly without completely forbidding wraparound.
For example, assuming that allocations happen in sequential numeric order (skipping in-use IDs), it could check if a freed ID is in a range where it would be reallocated soon, and if yes add it to a "deferred free" pool where IDs are freed only once the allocation ID safely passes them. This assumes that only a very small fraction of the ID space is used by extremely long-lived IDs, but this would have to be true anyway for managing limited resources.
Comment 1 by benhenry@google.com
, Jan 10