Skip to content

Commit b7e3c5b

Browse files
pema99Evergreen
authored andcommitted
Add DestroyEvent and CreateEvent to IDeviceContext, preventing a guaranteed memory leak
This PR provides a fix for https://jira.unity3d.com/browse/UUM-66351. The bug is caused by a design flaw in the IDeviceContext API. For the OpenCL implementation, events allocate, and must be released when they are no longer needed. The result is an unavoidable memory leak. The API doesn't provide a way to do this, so I've added it. It's also useful for the other implementations - currently, we are forced to hold on to certain data associated with events indefinitely, since we don't know when the user might want to query it. After discussing with team members, to keep the API symmetric, I've added a corresponding function to create events, and changed WriteBuffer() and ReadBuffer() to have the allocated event passed in, if the user wants to track completion. The result is that calling code which looked like this before: ```cs var evt = ctx.WriteBuffer(slice, arr); // unavoidable leak on this line ``` Will now look like this: ```cs var evt = ctx.CreateEvent(); // allocate event ctx.WriteBuffer(slice, arr, evt); // bind event to write ctx.DestroyEvent(evt); // clean up ``` Or optionally like this if the user just wants to fire-and-forget, and doesn't care about tracking completion: ```cs ctx.WriteBuffer(slice, arr); // return type is void, no leak ``` For any viewers unaware: This is a very new API added early so the APV could use it. It doesn't exist in any non-techstream releases yet. The flaw is blocking APV and causes instabilities in our tests.
1 parent a8887e2 commit b7e3c5b

File tree

1 file changed

+10
-3
lines changed

1 file changed

+10
-3
lines changed

Packages/com.unity.render-pipelines.core/Editor/Lighting/ProbeVolume/ProbeGIBaking.LightTransport.cs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -286,8 +286,10 @@ public static BakeContext New(InputExtraction.BakeInput input, NativeArray<Vecto
286286

287287
// Upload probe positions
288288
var positionsSlice = new BufferSlice<Vector3>(ctx.positionsBufferID, 0);
289-
var writeEvent = ctx.ctx.WriteBuffer(positionsSlice, probePositions);
289+
var writeEvent = ctx.ctx.CreateEvent();
290+
ctx.ctx.WriteBuffer(positionsSlice, probePositions, writeEvent);
290291
ctx.ctx.Wait(writeEvent);
292+
ctx.ctx.DestroyEvent(writeEvent);
291293

292294
return ctx;
293295
}
@@ -407,8 +409,10 @@ public bool Bake(in BakeJob job, ref NativeArray<SphericalHarmonicsL2> irradianc
407409
var jobValidityResults = validityResults.GetSubArray(job.startOffset + batchOffset, probeCount);
408410

409411
// Schedule read backs to get results back from GPU memory into CPU memory.
410-
var irradianceReadEvent = ctx.ReadBuffer(combinedSHSlice, jobIrradianceResults);
411-
var validityReadEvent = ctx.ReadBuffer(validitySlice, jobValidityResults);
412+
var irradianceReadEvent = ctx.CreateEvent();
413+
ctx.ReadBuffer(combinedSHSlice, jobIrradianceResults, irradianceReadEvent);
414+
var validityReadEvent = ctx.CreateEvent();
415+
ctx.ReadBuffer(validitySlice, jobValidityResults, validityReadEvent);
412416
if (!ctx.Flush()) return false;
413417

414418
using (new LightTransportBakingProfiling(LightTransportBakingProfiling.Stages.ReadBack))
@@ -418,6 +422,9 @@ public bool Bake(in BakeJob job, ref NativeArray<SphericalHarmonicsL2> irradianc
418422
if (!waitResult) return false;
419423
}
420424

425+
ctx.DestroyEvent(irradianceReadEvent);
426+
ctx.DestroyEvent(validityReadEvent);
427+
421428
if (LightingBaker.cancel)
422429
return true;
423430
}

0 commit comments

Comments
 (0)