Skip to content

Commit 184f42f

Browse files
committed
🦆🚫🪲
1 parent a5fb4da commit 184f42f

File tree

6 files changed

+181
-116
lines changed

6 files changed

+181
-116
lines changed

‎Source/DynamicProperties/MaterialPropertyManager.cs‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ private void OnDestroy()
3939
if (Instance != this) return;
4040

4141
Instance = null;
42-
PropsCascade.ClearCache();
42+
foreach (var cascade in rendererCascades.Values) cascade.Dispose();
43+
MpbCompilerCache.CheckCleared();
4344

4445
// Poor man's GC :'(
4546
MaterialColorUpdaterPatch.temperatureColorProps.Clear();
@@ -74,7 +75,9 @@ public bool Remove(Renderer renderer, Props props)
7475

7576
public bool Remove(Renderer renderer)
7677
{
77-
return rendererCascades.Remove(renderer);
78+
if (!rendererCascades.Remove(renderer, out var cascade)) return false;
79+
cascade.Dispose();
80+
return true;
7881
}
7982

8083
public static void RegisterPropertyNamesForDebugLogging(params string[] properties)
@@ -85,15 +88,9 @@ public static void RegisterPropertyNamesForDebugLogging(params string[] properti
8588
#endregion
8689

8790
/// Public API equivalent is calling `Props.Dispose`.
88-
internal bool Remove(Props props)
91+
internal void Remove(Props props)
8992
{
90-
var removed = false;
91-
92-
foreach (var cascade in rendererCascades.Values) removed |= cascade.Remove(props);
93-
94-
PropsCascade.RemoveCacheEntriesWith(props);
95-
96-
return removed;
93+
foreach (var cascade in rendererCascades.Values) cascade.Remove(props);
9794
}
9895

9996
private bool _propRefreshScheduled = false;
@@ -105,9 +102,9 @@ private IEnumerator<YieldInstruction> Co_propsLateUpdate()
105102

106103
foreach (var props in propsLateUpdateQueue) {
107104
if (props.NeedsEntriesUpdate) {
108-
props.OnEntriesChanged(props);
105+
props.OnEntriesChanged?.Invoke(props);
109106
} else if (props.NeedsValueUpdate) {
110-
props.OnValueChanged(props, null);
107+
props.OnValueChanged?.Invoke(props, null);
111108
}
112109

113110
props.SuppressEagerUpdate =

‎Source/DynamicProperties/MpbCompiler.cs‎

Lines changed: 23 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,18 @@ internal class MpbCompiler : IDisposable
1717

1818
private readonly HashSet<Renderer> linkedRenderers = [];
1919
private readonly MaterialPropertyBlock mpb = new();
20-
private readonly Dictionary<Props, List<int>> idManagerMap = [];
20+
private readonly Dictionary<int, Props> idManagers = [];
2121

2222
private static readonly MaterialPropertyBlock EmptyMpb = new();
2323

2424
#endregion
2525

26-
internal MpbCompiler(SortedSet<Props> cascades)
26+
internal MpbCompiler(SortedSet<Props> cascade)
2727
{
2828
MaterialPropertyManager.Instance?.LogDebug(
29-
$"new cache entry {RuntimeHelpers.GetHashCode(this)}");
29+
$"new MpbCompiler instance {RuntimeHelpers.GetHashCode(this)}");
3030

31-
Cascade = cascades;
31+
Cascade = cascade;
3232
RebuildManagerMap();
3333
RewriteMpb();
3434
foreach (var props in Cascade) {
@@ -48,16 +48,12 @@ internal void Register(Renderer renderer)
4848
internal void Unregister(Renderer renderer)
4949
{
5050
linkedRenderers.Remove(renderer);
51-
renderer.SetPropertyBlock(EmptyMpb);
52-
CheckLiveness();
53-
}
51+
if (renderer != null) renderer.SetPropertyBlock(EmptyMpb);
5452

55-
private void CheckLiveness()
56-
{
5753
if (linkedRenderers.Count > 0) return;
58-
MaterialPropertyManager.Instance.LogDebug(
59-
$"dead cache entry {RuntimeHelpers.GetHashCode(this)}");
60-
PropsCascade.RemoveCacheEntry(this);
54+
Log.Debug(
55+
$"last renderer unregistered from MpbCompiler instance {RuntimeHelpers.GetHashCode(this)}");
56+
MpbCompilerCache.Remove(this);
6157
}
6258

6359
#endregion
@@ -66,22 +62,12 @@ private void CheckLiveness()
6662

6763
private void RebuildManagerMap()
6864
{
69-
idManagerMap.Clear();
70-
71-
Dictionary<int, Props> idManagers = [];
65+
idManagers.Clear();
7266
foreach (var props in Cascade) {
7367
foreach (var id in props.ManagedIds) {
7468
idManagers[id] = props;
7569
}
7670
}
77-
78-
foreach (var (id, props) in idManagers) {
79-
if (!idManagerMap.TryGetValue(props, out var ids)) {
80-
idManagerMap[props] = ids = [];
81-
}
82-
83-
ids.Add(id);
84-
}
8571
}
8672

8773
private void OnPropsValueChanged(Props props, int? id)
@@ -105,21 +91,22 @@ private void OnPropsEntriesChanged(Props props)
10591
private void WriteMpb(Props props, int? id)
10692
{
10793
if (id.HasValue) {
108-
props.Write(id.GetValueOrDefault(), mpb);
94+
var changedId = id.GetValueOrDefault();
95+
if (idManagers[changedId] != props) return;
96+
props.Write(changedId, mpb);
10997
} else {
110-
foreach (var managedId in idManagerMap[props]) props.Write(managedId, mpb);
98+
foreach (var (managedId, managingProps) in idManagers) {
99+
if (props != managingProps) continue;
100+
props.Write(managedId, mpb);
101+
}
111102
}
112103
}
113104

114105
[MethodImpl(MethodImplOptions.AggressiveInlining)]
115106
private void RewriteMpb()
116107
{
117108
mpb.Clear();
118-
foreach (var (props, managedIds) in idManagerMap) {
119-
foreach (var managedId in managedIds) {
120-
props.Write(managedId, mpb);
121-
}
122-
}
109+
foreach (var (id, props) in idManagers) props.Write(id, mpb);
123110
}
124111

125112
[MethodImpl(MethodImplOptions.AggressiveInlining)]
@@ -143,7 +130,7 @@ private void ApplyAll()
143130
MaterialPropertyManager.Instance.Remove(dead);
144131
}
145132

146-
CheckLiveness();
133+
_deadRenderers.Clear();
147134
}
148135

149136
#endregion
@@ -152,29 +139,29 @@ private void ApplyAll()
152139

153140
private bool _disposed = false;
154141

155-
private void UnlinkProps()
142+
private void HandleDispose()
156143
{
157144
if (_disposed) return;
158145

159-
Debug.Log("disposing MPB cache entry");
146+
Log.Debug($"disposing MPB compiler instance {RuntimeHelpers.GetHashCode(this)}");
160147

161148
foreach (var props in Cascade) {
162-
props.OnValueChanged -= OnPropsValueChanged;
163149
props.OnEntriesChanged -= OnPropsEntriesChanged;
150+
props.OnValueChanged -= OnPropsValueChanged;
164151
}
165152

166153
_disposed = true;
167154
}
168155

169156
public void Dispose()
170157
{
171-
UnlinkProps();
158+
HandleDispose();
172159
GC.SuppressFinalize(this);
173160
}
174161

175162
~MpbCompiler()
176163
{
177-
UnlinkProps();
164+
HandleDispose();
178165
}
179166

180167
#endregion
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Runtime.CompilerServices;
4+
using KSPBuildTools;
5+
using UnityEngine;
6+
7+
namespace Shabby.DynamicProperties;
8+
9+
internal static class MpbCompilerCache
10+
{
11+
private static readonly IEqualityComparer<SortedSet<Props>> CacheKeyComparer =
12+
SortedSet<Props>.CreateSetComparer(); // Object equality is fine.
13+
14+
private static readonly Dictionary<SortedSet<Props>, MpbCompiler> Cache =
15+
new(CacheKeyComparer);
16+
17+
internal static MpbCompiler Get(SortedSet<Props> cascade)
18+
{
19+
if (Cache.TryGetValue(cascade, out var compiler)) {
20+
MaterialPropertyManager.Instance?.LogDebug(
21+
$"MpbCompiler cache hit instance {RuntimeHelpers.GetHashCode(compiler)}");
22+
return compiler;
23+
}
24+
25+
// Don't accidentally mutate the cache key...
26+
var clonedCascade = new SortedSet<Props>(cascade);
27+
compiler = new MpbCompiler(clonedCascade);
28+
#if DEBUG
29+
if (!(!ReferenceEquals(cascade, compiler.Cascade) &&
30+
CacheKeyComparer.Equals(cascade, compiler.Cascade))) {
31+
throw new InvalidOperationException("cache key equality check failed");
32+
}
33+
#endif
34+
Cache[compiler.Cascade] = compiler;
35+
return compiler;
36+
}
37+
38+
internal static void Remove(MpbCompiler entry)
39+
{
40+
Cache.Remove(entry.Cascade);
41+
entry.Dispose();
42+
}
43+
44+
internal static void CheckCleared()
45+
{
46+
if (Cache.Count == 0) return;
47+
48+
Debug.LogError($"{Cache.Count} MpbCompilers were not disposed; forcing removal");
49+
foreach (var compiler in Cache.Values) compiler.Dispose();
50+
Cache.Clear();
51+
}
52+
}

‎Source/DynamicProperties/Props.cs‎

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,16 @@
66

77
namespace Shabby.DynamicProperties;
88

9-
public sealed class Props : IDisposable
9+
public sealed class Props : IComparable<Props>, IDisposable
1010
{
1111
/// Ordered by lowest to highest priority. Equal priority is disambiguated by unique IDs.
12-
/// Note that this is compatible with default object reference equality.
13-
public static readonly Comparer<Props> PriorityComparer = Comparer<Props>.Create((a, b) =>
12+
public int CompareTo(Props other)
1413
{
15-
var priorityCmp = a.Priority.CompareTo(b.Priority);
16-
return priorityCmp != 0 ? priorityCmp : a.UniqueId.CompareTo(b.UniqueId);
17-
});
14+
if (ReferenceEquals(this, other)) return 0;
15+
if (other is null) return 1;
16+
var priorityCmp = Priority.CompareTo(other.Priority);
17+
return priorityCmp != 0 ? priorityCmp : UniqueId.CompareTo(other.UniqueId);
18+
}
1819

1920
private static uint _idCounter = 0;
2021
private static uint _nextId() => _idCounter++;
@@ -27,16 +28,17 @@ public sealed class Props : IDisposable
2728

2829
internal IEnumerable<int> ManagedIds => props.Keys;
2930

30-
internal delegate void ValueChangedHandler(Props props, int? id);
31-
3231
internal delegate void EntriesChangedHandler(Props props);
3332

34-
internal ValueChangedHandler OnValueChanged = delegate { };
3533
internal EntriesChangedHandler OnEntriesChanged = delegate { };
3634

35+
internal delegate void ValueChangedHandler(Props props, int? id);
36+
37+
internal ValueChangedHandler OnValueChanged = delegate { };
38+
3739
internal bool SuppressEagerUpdate = false;
38-
internal bool NeedsValueUpdate = false;
3940
internal bool NeedsEntriesUpdate = false;
41+
internal bool NeedsValueUpdate = false;
4042

4143
public Props(int priority)
4244
{
@@ -58,7 +60,7 @@ private void _internalSet<T, TProp>(int id, T value) where TProp : Prop<T>
5860
if (!typedProp.UpdateIfChanged(value)) return;
5961

6062
if (!SuppressEagerUpdate) {
61-
OnValueChanged(this, id);
63+
OnValueChanged?.Invoke(this, id);
6264
} else {
6365
NeedsValueUpdate = true;
6466
}
@@ -73,7 +75,7 @@ private void _internalSet<T, TProp>(int id, T value) where TProp : Prop<T>
7375
props[id] = (TProp)Activator.CreateInstance(typeof(TProp), value);
7476

7577
if (!SuppressEagerUpdate) {
76-
OnEntriesChanged(this);
78+
OnEntriesChanged?.Invoke(this);
7779
} else {
7880
NeedsEntriesUpdate = true;
7981
}
@@ -126,22 +128,28 @@ public override string ToString()
126128

127129
private bool _disposed = false;
128130

129-
private void UnregisterSelf(bool disposing)
131+
private void HandleDispose(bool disposing)
130132
{
131133
if (_disposed) return;
132-
Debug.Log($"disposing Props instance {UniqueId}");
133-
if (disposing) MaterialPropertyManager.Instance?.Remove(this);
134+
135+
if (disposing) {
136+
Log.Debug($"disposing Props instance {UniqueId}");
137+
MaterialPropertyManager.Instance?.Remove(this);
138+
} else {
139+
Log.Error($"Props instance {UniqueId} was not disposed");
140+
}
141+
134142
_disposed = true;
135143
}
136144

137145
public void Dispose()
138146
{
139-
UnregisterSelf(true);
147+
HandleDispose(true);
140148
GC.SuppressFinalize(this);
141149
}
142150

143151
~Props()
144152
{
145-
UnregisterSelf(false);
153+
HandleDispose(false);
146154
}
147155
}

0 commit comments

Comments
 (0)