Skip to content

Commit

Permalink
fix: dispose TextureFrame references instantly (#981)
Browse files Browse the repository at this point in the history
* fix: TextureFrame is not disposed instantly

* fix: remove keys out of the loop

* feat: implement GlobalInstanceTable.count

* test: add GlobalInstanceTable tests
  • Loading branch information
homuler committed Aug 5, 2023
1 parent 514a0a9 commit 678596f
Show file tree
Hide file tree
Showing 6 changed files with 282 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,11 @@ public void Dispose()
var name = (uint)_nativeTexturePtr;
lock (((ICollection)_NameTable).SyncRoot)
{
var _ = _NameTable.Remove(name);
_ = _NameTable.Remove(name);
}
}
_glSyncToken?.Dispose();
_ = _InstanceTable.Remove(_instanceId);
}

public void CopyTexture(Texture dst) => Graphics.CopyTexture(_texture, dst);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,17 @@ public TextureFramePool(int textureWidth, int textureHeight, TextureFormat textu
_textureFramesInUse = new Dictionary<Guid, TextureFrame>(poolSize);
}

void IDisposable.Dispose()
public void Dispose()
{
_textureFramesLock.EnterWriteLock();
try
{
foreach (var textureFrame in _availableTextureFrames)
{
textureFrame.Dispose();
}
_availableTextureFrames.Clear();

foreach (var textureFrame in _textureFramesInUse.Values)
{
textureFrame.Dispose();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,15 @@
using System;
using System.Collections.Generic;
using System.Threading;
using System.Linq;

namespace Mediapipe
{
// TODO: make GlobalInstanceTable internal
public class GlobalInstanceTable<TKey, TValue> where TValue : class
{
private readonly ReaderWriterLockSlim _tableLock = new ReaderWriterLockSlim();
private readonly Dictionary<TKey, WeakReference<TValue>> _table = new Dictionary<TKey, WeakReference<TValue>>();
private readonly Dictionary<TKey, WeakReference<TValue>> _table;

private int _maxSize;
/// <summary>
Expand All @@ -35,9 +36,26 @@ public int maxSize
}
}

public int count
{
get
{
_tableLock.EnterReadLock();
try
{
return _table.Count;
}
finally
{
_tableLock.ExitReadLock();
}
}
}

public GlobalInstanceTable(int maxSize = 0)
{
this.maxSize = maxSize;
_table = new Dictionary<TKey, WeakReference<TValue>>(maxSize);
}

public void Add(TKey key, TValue value)
Expand Down Expand Up @@ -118,17 +136,29 @@ public bool ContainsKey(TKey key)
}
}

public bool Remove(TKey key)
{
_tableLock.EnterWriteLock();
try
{
return _table.Remove(key);
}
finally
{
_tableLock.ExitWriteLock();
}
}

/// <remarks>
/// Aquire the write lock before calling this method.
/// </remarks>
private void ClearUnusedKeys()
{
foreach (var pair in _table)
var deadKeys = _table.Where(x => !x.Value.TryGetTarget(out var target)).Select(x => x.Key).ToArray();

foreach (var key in deadKeys)
{
if (!pair.Value.TryGetTarget(out var _))
{
var _ = _table.Remove(pair.Key);
}
var _ = _table.Remove(key);
}
}
}
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,219 @@
// Copyright (c) 2023 homuler
//
// Use of this source code is governed by an MIT-style
// license that can be found in the LICENSE file or at
// https://opensource.org/licenses/MIT.

using System;
using NUnit.Framework;

namespace Mediapipe.Tests.Util
{
public class GlobalInstanceTableTest
{
private class Value
{
public readonly int value;

public Value(int value)
{
this.value = value;
}
}

#region Constructor
[Test]
public void Ctor_Throws_WhenMaxSizeIsLessThenZero() => Assert.Throws<ArgumentException>(() => new GlobalInstanceTable<int, Value>(-1));

[Test]
public void Ctor_InstantiateAnEmptyTable_WhenMaxSizeIsZero()
{
var table = new GlobalInstanceTable<int, Value>();
Assert.AreEqual(0, table.maxSize);
}

[Test]
public void Ctor_InstantiateAnEmptyTable_WhenMaxSizeIsSpecified()
{
var table = new GlobalInstanceTable<int, Value>(10);
Assert.AreEqual(10, table.maxSize);
}
#endregion

#region maxSize
[Test]
public void MaxSize_MustBeLargerThanZero()
{
var table = new GlobalInstanceTable<int, Value>(10);
Assert.Throws<ArgumentException>(() => table.maxSize = -1);
}

[Test]
public void MaxSize_CanBeChangedToLargerValue()
{
var table = new GlobalInstanceTable<int, Value>(5);
Assert.AreEqual(5, table.maxSize);

table.maxSize = 6;
Assert.AreEqual(6, table.maxSize);
}

[Test]
public void MaxSize_CanBeChangedToSmallerValue()
{
var table = new GlobalInstanceTable<int, Value>(5);
Assert.AreEqual(5, table.maxSize);

table.Add(1, new Value(1));
table.Add(2, new Value(2));
table.Add(3, new Value(3));
table.Add(4, new Value(4));
table.Add(5, new Value(5));

table.maxSize = 2;
Assert.AreEqual(2, table.maxSize);
Assert.AreEqual(5, table.count);
}
#endregion

#region Add
[Test]
public void CannotAdd_IfCountEqualsMaxSize()
{
var table = new GlobalInstanceTable<int, Value>(1);
var v1 = new Value(1);

table.Add(1, v1);
Assert.Throws<InvalidOperationException>(() => table.Add(2, new Value(2)));

GC.KeepAlive(v1);
}

[Test, Ignore("Skip because it's non-deterministic")]
public void CanAdd_IfCountEqualsMaxSize_But_SomeValuesAreGCed()
{
var table = new GlobalInstanceTable<int, Value>(1);

table.Add(1, new Value(1));
GC.Collect();

Assert.DoesNotThrow(() => table.Add(2, new Value(2)));
}

[Test]
public void CannotAdd_If_KeyAlreadyExists()
{
var table = new GlobalInstanceTable<int, Value>(2);
var v1 = new Value(1);

table.Add(1, v1);
Assert.Throws<ArgumentException>(() => table.Add(1, new Value(2)));

GC.KeepAlive(v1);
}

[Test, Ignore("Skip because it's non-deterministic")]
public void CanAdd_If_KeyAlreadyExists_But_TheReferenceIsGCed()
{
var table = new GlobalInstanceTable<int, Value>(2);

table.Add(1, new Value(1));
GC.Collect();

Assert.DoesNotThrow(() => table.Add(1, new Value(2)));
}
#endregion

#region TryGetValue
[Test]
public void TryGetValue_ReturnsTrue_IfTheKeyExists()
{
var table = new GlobalInstanceTable<int, Value>(1);
var v1 = new Value(1);

table.Add(1, v1);
Assert.IsTrue(table.TryGetValue(1, out var v2));
Assert.AreEqual(v1, v2);

GC.KeepAlive(v1);
}

[Test, Ignore("Skip because it's non-deterministic")]
public void TryGetValue_ReturnsFalse_IfTheKeyExists_But_TheValueIsGCed()
{
var table = new GlobalInstanceTable<int, Value>(1);

table.Add(1, new Value(1));
GC.Collect();

Assert.IsFalse(table.TryGetValue(1, out var _));
}

[Test]
public void TryGetValue_ReturnsFalse_IfTheKeyDoesNotExist()
{
var table = new GlobalInstanceTable<int, Value>(1);
Assert.IsFalse(table.TryGetValue(1, out var _));
}
#endregion

#region Clear
[Test]
public void Clear_ClearsTheTable()
{
var table = new GlobalInstanceTable<int, Value>(2);
var v1 = new Value(1);
var v2 = new Value(2);

table.Add(1, v1);
table.Add(2, v2);
Assert.AreEqual(2, table.count);

table.Clear();
Assert.AreEqual(0, table.count);

GC.KeepAlive(v1);
GC.KeepAlive(v2);
}
#endregion

#region ContainsKey
[Test, Ignore("Skip because it's non-deterministic")]
public void ContainsKey_ReturnsTrue_IfTheKeyExists()
{
var table = new GlobalInstanceTable<int, Value>(1);
table.Add(1, new Value(1));

GC.Collect();

Assert.IsTrue(table.ContainsKey(1));
Assert.False(table.TryGetValue(1, out var _));
}

[Test]
public void ContainsKey_ReturnsFalse_IfTheKyeDoesNotExist()
{
var table = new GlobalInstanceTable<int, Value>(1);
Assert.IsFalse(table.ContainsKey(1));
}
#endregion

#region Remove
[Test]
public void Remove_RemovesTheKeyFromTheTable()
{
var table = new GlobalInstanceTable<int, Value>(1);
var v1 = new Value(1);

table.Add(1, v1);
Assert.AreEqual(1, table.count);

table.Remove(1);
Assert.IsFalse(table.ContainsKey(1));
Assert.AreEqual(0, table.count);

GC.KeepAlive(v1);
}
#endregion
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 678596f

Please sign in to comment.