Bug Memory leak in HybridV2RenderSystem.AddNewChunks(0.50.0-preview.44)

Memory leak when AddNewBatch fail.
This is my temporary solution:

Code (CSharp):

1.  
2.         private int AddNewChunks(NativeArray<ArchetypeChunk> newChunksX,
3.             //Modify!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
4.             out NativeArray<ArchetypeChunk> sortedNewChunks)
5.         {
6.             int numValidNewChunks = 0;
7.  
8.             Debug.Assert(newChunksX.Length > 0, "Attempted to add new chunks, but list of new chunks was empty");
9.  
10.             var hybridChunkInfoType = GetComponentTypeHandle<HybridChunkInfo>();
11.             // Sort new chunks by RenderMesh so we can put
12.             // all compatible chunks inside one batch.
13.             var batchCreateInfoFactory = new BatchCreateInfoFactory
14.             {
15.                 EntityManager = EntityManager,
16.                 RenderMeshTypeHandle = GetSharedComponentTypeHandle<RenderMesh>(),
17.                 EditorRenderDataTypeHandle = GetSharedComponentTypeHandle<EditorRenderData>(),
18.                 HybridBatchPartitionHandle = GetSharedComponentTypeHandle<HybridBatchPartition>(),
19.                 RenderMeshFlippedWindingTagTypeHandle = GetComponentTypeHandle<RenderMeshFlippedWindingTag>(),
20. #if UNITY_EDITOR
21.                 DefaultEditorRenderData = new EditorRenderData
22.                 {SceneCullingMask = UnityEditor.SceneManagement.EditorSceneManager.DefaultSceneCullingMask},
23. #else
24.                 DefaultEditorRenderData = new EditorRenderData { SceneCullingMask = ~0UL },
25. #endif
26.             };
27.             var batchCompatibility = new BatchCompatibility(this, batchCreateInfoFactory, newChunksX);
28.             // This also sorts invalid chunks to the back.
29.             sortedNewChunks = batchCompatibility.SortChunks();
30.  
31.             int batchBegin = 0;
32.             int numInstances = sortedNewChunks[0].Capacity;
33.  
34.             for (int i = 1; i <= sortedNewChunks.Length; ++i)
35.             {
36.                 int instancesInChunk = 0;
37.                 bool breakBatch = false;
38.  
39.                 if (i < sortedNewChunks.Length)
40.                 {
41.                     var chunk = sortedNewChunks[i];
42.                     breakBatch = !batchCompatibility.IsCompatible(EntityManager, this, batchBegin, i);
43.                     instancesInChunk = chunk.Capacity;
44.                 }
45.                 else
46.                 {
47.                     breakBatch = true;
48.                 }
49.  
50.                 if (numInstances + instancesInChunk > kMaxEntitiesPerBatch)
51.                     breakBatch = true;
52.  
53.                 if (breakBatch)
54.                 {
55.                     int numChunks = i - batchBegin;
56.  
57.                     var createInfo = batchCompatibility.CreateInfoFor(batchBegin);
58.                     bool valid = AddNewBatch(ref createInfo, ref hybridChunkInfoType,
59.                         sortedNewChunks.GetSubArray(batchBegin, numChunks), numInstances);
60.  
61.                     // As soon as we encounter an invalid chunk, we know that all the rest are invalid
62.                     // too.
63.                     if (valid)
64.                         numValidNewChunks += numChunks;
65.                     else
66.                     {
67.                         //Modify!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
68.                         batchCompatibility.Dispose();
69.                         //sortedNewChunks.Dispose();
70.  
71.                         return numValidNewChunks;
72.                     }
73.  
74.                     batchBegin = i;
75.                     numInstances = instancesInChunk;
76.                 }
77.                 else
78.                 {
79.                     numInstances += instancesInChunk;
80.                 }
81.             }
82.  
83.             batchCompatibility.Dispose();
84.  
85.             //Modify!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
86.             //sortedNewChunks.Dispose();
87.  
88.             return numValidNewChunks;
89.         }
90.  

Code (CSharp):

1.  
2.             public NativeArray<ArchetypeChunk> SortChunks()
3.             {
4.                 // Key-value sort all chunks according to compatibility
5.                 m_SortIndices.Sort(this);
6.                 var sortedChunks = new NativeArray<ArchetypeChunk>(
7.                     m_Chunks.Length,
8.                     //Modify!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
9.                     Allocator.TempJob,
10.                     NativeArrayOptions.UninitializedMemory);
11.  
12.                 for (int i = 0; i < m_Chunks.Length; ++i)
13.                     sortedChunks[i] = m_Chunks[SortedIndex(i)];
14.  
15.                 return sortedChunks;
16.             }
17.  

Code (CSharp):

1.  
2.             if (numNewChunks > 0)
3.             {
4.                 Profiler.BeginSample("AddNewChunks");
5.                 int numValidNewChunks = AddNewChunks(newChunks.GetSubArray(0, numNewChunks), out var sortedNewChunks);
6.                 Profiler.EndSample();
7.  
8.                 hybridChunkUpdater.PreviousBatchIndex = -1;
9.  
10.                 var updateNewChunksJob = new UpdateNewHybridChunksJob
11.                 {
12.                     //Modify!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
13.                     NewChunks = sortedNewChunks,
14.                     HybridChunkInfo = hybridRenderedChunkType,
15.                     ChunkWorldRenderBounds = chunkWorldRenderBoundsRO,
16.                     HybridChunkUpdater = hybridChunkUpdater,
17.                 };
18.  
19. #if DEBUG_LOG_INVALID_CHUNKS
20.                 if (numValidNewChunks != numNewChunks)
21.                     Debug.Log($"Tried to add {numNewChunks} new chunks, but only {numValidNewChunks} were valid, {numNewChunks - numValidNewChunks} were invalid");
22. #endif
23.  
24.                 hybridCompleted = updateNewChunksJob.Schedule(numValidNewChunks, kNumNewChunksPerThread);
25.             }
26.  

Code (CSharp):

1.  
2.     [BurstCompile]
3.     internal struct UpdateNewHybridChunksJob : IJobParallelFor
4.     {
5.         public ComponentTypeHandle<HybridChunkInfo> HybridChunkInfo;
6.         [ReadOnly] public ComponentTypeHandle<ChunkWorldRenderBounds> ChunkWorldRenderBounds;
7.  
8.         //Modify!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
9.         [DeallocateOnJobCompletion]
10.         public NativeArray<ArchetypeChunk> NewChunks;
11.         public HybridChunkUpdater HybridChunkUpdater;
12.  
13.         public void Execute(int index)
14.         {
15.             var chunk = NewChunks[index];
16.             var chunkInfo = chunk.GetChunkComponentData(HybridChunkInfo);
17.  
18.             ChunkWorldRenderBounds chunkBounds = chunk.GetChunkComponentData(ChunkWorldRenderBounds);
19.  
20.             Debug.Assert(chunkInfo.Valid, "Attempted to process a chunk with uninitialized Hybrid chunk info");
21.             HybridChunkUpdater.MarkBatchForUpdates(chunkInfo.InternalIndex, true);
22.             HybridChunkUpdater.ProcessValidChunk(ref chunkInfo, chunk, chunkBounds.Value, true);
23.             chunk.SetChunkComponentData(HybridChunkInfo, chunkInfo);
24.             HybridChunkUpdater.FinishExecute();
25.         }
26.     }
27.  

 

batchCompatibility and sortedNewChunks both appear to be allocated with Allocator.Temp, meaning that calling Dispose on them is optional and is essentially a no-op. I don't see how there is a memory leak in the original code.

And even if there were, the better solution would be to replace the allocations with World.UpdateAllocator. Like temp, those also don't need to be Disposed, but unlike Temp, you can pass them in and out of jobs and they have a lifecycle of 2 world updates (2 frames).

 

Maybe Allocator.Temp calling Dispose is optional in burst,but it crash in my test.
And the UpdateNewHybridChunksJob.NewChunks is not a sorted chunk list,it's HybridChunkInfo can get a invail value.

My English is poor,but trust me in the code, it's a bug from 0.11.0-preview.42.

 

I see now. Let me make a few things clear.
1) This is a bug that can cause crashes.
2) This is not a memory leak.
3) This only happens when the Mesh, Material, or Shader of a RenderMesh is null.

Here's how I am choosing to fix it. Rather than juggle a new container, I just update the existing container to reflect the assumptions made on it. https://github.com/Dreaming381/Kine...mmit/79383f9754461a72291c971bdb9a8e2496189243

 

That's ok in my case.
And i found other crash bug when AddNewChunks after RemoveBatch ,I can't tell you why by my poor English,but i can show you how to fix it:

Code (CSharp):

1.  
2.         internal int InternalIndexRange = 1;// => m_SortedInternalIds.Max + 1;
3.  

Code (CSharp):

1.  
2.         internal int AllocateInternalId()
3.         {
4.             EnsureHaveSpaceForNewBatch();
5.  
6.             int id = m_InternalIdFreelist[m_InternalIdFreelist.Length - 1];
7.  
8.             m_InternalIdFreelist.Resize(m_InternalIdFreelist.Length - 1, NativeArrayOptions.UninitializedMemory);
9.  
10.             Debug.Assert(!m_SortedInternalIds.Contains(id), "Freshly allocated batch id found in list of used ids");
11.  
12.             m_SortedInternalIds.Add(id);
13.  
14.             //Modity!!!!!!!!!!!!!!!!!!!!!!!!!!!!
15.             InternalIndexRange = math.max(InternalIndexRange, id + 1);
16.  
17.             return id;
18.         }
19.  

 

Can you at least tell me what line the crash happens so I can figure out why?

 

in public JobHandle UpdateAllBatches(JobHandle inputDependencies)

Code (CSharp):

1.  
2.             var maxBatchCount = math.max(kInitialMaxBatchCount, InternalIndexRange + numNewChunks);
3.  
4.             // Integer division with round up
5.             var maxBatchLongCount = (maxBatchCount + kNumBitsPerLong - 1) / kNumBitsPerLong;
6.  
7.             var batchRequiresUpdates = new NativeArray<long>(
8.                 maxBatchLongCount,
9.                 Allocator.TempJob,
10.                 NativeArrayOptions.ClearMemory);
11.  
12.             var batchHadMovingEntities = new NativeArray<long>(
13.                 maxBatchLongCount,
14.                 Allocator.TempJob,
15.                 NativeArrayOptions.ClearMemory);

Sometime maxBatchCount < chunkInfo.InternalIndex,and crash in "Batch index out of bounds":

Code (CSharp):

1.  
2.         public unsafe void MarkBatchForUpdates(int internalIndex, bool entitiesMoved)
3.         {
4.             AtomicHelpers.IndexToQwIndexAndMask(internalIndex, out int qw, out long mask);
5.             if(qw >= BatchRequiresUpdates.Length || qw >= BatchHadMovingEntities.Length)
6.             Debug.Assert(qw < BatchRequiresUpdates.Length && qw < BatchHadMovingEntities.Length,
7.                 "Batch index out of bounds");
8.  
9.             var motionInfo = BatchMotionInfos[internalIndex];
10.             bool mustDisableMotionVectors = motionInfo.MotionVectorFlagSet && !entitiesMoved;
11.  
12.             // If entities moved, we always update the batch since bounds must be updated.
13.             // If no entities moved, we only update the batch if it requires motion vector disable.
14.             if (entitiesMoved || mustDisableMotionVectors)
15.                 AtomicHelpers.AtomicOr((long*)BatchRequiresUpdates.GetUnsafePtr(), qw, mask);
16.  
17.             if (entitiesMoved)
18.                 AtomicHelpers.AtomicOr((long*)BatchHadMovingEntities.GetUnsafePtr(), qw, mask);
19.         }
20.