From 32f4aa3e35a12cac8f3a31b1f0b9bb8b4324829c Mon Sep 17 00:00:00 2001 From: hypercross Date: Sat, 18 Jul 2026 21:42:08 +0800 Subject: [PATCH] refactor: optimize query execution and improve change tracking - Optimize `QueryExecutor` to use the smallest sparse set as a driver for multi-component queries, reducing iteration overhead. - Implement automatic cleanup of unused `Subject` instances in `ChangeBuffer` using a subscriber count. - Improve `ChangeSet` deduplication to correctly handle rapid add/remove transitions for the same component. - Add validation to ensure `IRelationship` components are added to their declared `Source` entity. - Update documentation to reflect correct `Entity` bit layout. --- docs/architecture.md | 4 +- src/OECS/ChangeBuffer.cs | 84 ++++++-- src/OECS/ChangeSet.cs | 33 +++- src/OECS/QueryExecutor.cs | 389 +++++++++++++++++++++++++++++++++----- src/OECS/World.cs | 11 ++ 5 files changed, 450 insertions(+), 71 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index 2c2e024..f13f059 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -45,8 +45,8 @@ signature) and sparse sets (one dense array per component type). **Context:** Entities need to be cheap to copy, comparable, and safe against use-after-free (accessing a recycled entity ID). -**Decision:** `readonly struct Entity` wrapping a `uint`. Upper 24 bits are the -ID, lower 8 bits are the version. +**Decision:** `readonly struct Entity` wrapping a `uint`. Lower 24 bits are the +ID, upper 8 bits are the version. **Rationale:** diff --git a/src/OECS/ChangeBuffer.cs b/src/OECS/ChangeBuffer.cs index c25842e..1f77414 100644 --- a/src/OECS/ChangeBuffer.cs +++ b/src/OECS/ChangeBuffer.cs @@ -14,8 +14,18 @@ internal class ChangeBuffer private readonly ChangeSet _pending = new(); private readonly Subject _entitySubject = new(); - private readonly Dictionary> _componentSubjects = new(); - private readonly Dictionary> _querySubjects = new(); + private readonly Dictionary _componentSubjects = new(); + private readonly Dictionary _querySubjects = new(); + + /// + /// Wraps a with a subscriber count so that + /// subjects with no remaining subscribers can be cleaned up. + /// + private sealed class TrackedSubject + { + public readonly Subject Subject = new(); + public int SubscriberCount; + } /// /// The change set currently accumulating. Cleared after each . @@ -40,18 +50,18 @@ internal class ChangeBuffer // Push to component-specific subjects. if (change.ComponentType != null) { - if (_componentSubjects.TryGetValue(change.ComponentType, out var compSubject)) + if (_componentSubjects.TryGetValue(change.ComponentType, out var compTracked)) { - compSubject.OnNext(change); + compTracked.Subject.OnNext(change); } } // Push to matching query subjects. - foreach (var (query, subject) in _querySubjects) + foreach (var (query, tracked) in _querySubjects) { if (ChangeMatchesQuery(change, query)) { - subject.OnNext(change); + tracked.Subject.OnNext(change); } } } @@ -72,12 +82,22 @@ internal class ChangeBuffer /// public Observable ObserveComponentChanges(Type componentType) { - if (!_componentSubjects.TryGetValue(componentType, out var subject)) + if (!_componentSubjects.TryGetValue(componentType, out var tracked)) { - subject = new Subject(); - _componentSubjects[componentType] = subject; + tracked = new TrackedSubject(); + _componentSubjects[componentType] = tracked; } - return subject; + + tracked.SubscriberCount++; + return WrapWithCleanup(tracked.Subject, () => + { + tracked.SubscriberCount--; + if (tracked.SubscriberCount <= 0) + { + tracked.Subject.Dispose(); + _componentSubjects.Remove(componentType); + } + }); } /// @@ -85,12 +105,40 @@ internal class ChangeBuffer /// public Observable ObserveQuery(QueryDescriptor query) { - if (!_querySubjects.TryGetValue(query, out var subject)) + if (!_querySubjects.TryGetValue(query, out var tracked)) { - subject = new Subject(); - _querySubjects[query] = subject; + tracked = new TrackedSubject(); + _querySubjects[query] = tracked; } - return subject; + + tracked.SubscriberCount++; + return WrapWithCleanup(tracked.Subject, () => + { + tracked.SubscriberCount--; + if (tracked.SubscriberCount <= 0) + { + tracked.Subject.Dispose(); + _querySubjects.Remove(query); + } + }); + } + + /// + /// Wraps an observable so that is called + /// when the last subscriber disposes. + /// + private static Observable WrapWithCleanup( + Observable source, Action onLastDispose) + { + return Observable.Create(observer => + { + var subscription = source.Subscribe(observer); + return Disposable.Create(() => + { + subscription.Dispose(); + onLastDispose(); + }); + }); } /// @@ -99,14 +147,14 @@ internal class ChangeBuffer public void Dispose() { _entitySubject.Dispose(); - foreach (var subject in _componentSubjects.Values) + foreach (var tracked in _componentSubjects.Values) { - subject.Dispose(); + tracked.Subject.Dispose(); } _componentSubjects.Clear(); - foreach (var subject in _querySubjects.Values) + foreach (var tracked in _querySubjects.Values) { - subject.Dispose(); + tracked.Subject.Dispose(); } _querySubjects.Clear(); } diff --git a/src/OECS/ChangeSet.cs b/src/OECS/ChangeSet.cs index 0c3b0f7..a1795bd 100644 --- a/src/OECS/ChangeSet.cs +++ b/src/OECS/ChangeSet.cs @@ -4,13 +4,14 @@ namespace OECS; /// Accumulates entries during a system run. /// /// Deduplication: if the same (entity, kind, componentType) change is marked -/// multiple times, only one entry is kept. Entity-level changes (Added/Removed) -/// take precedence over component-level changes for the same entity. +/// multiple times, only one entry is kept. However, if a structurally opposed +/// change arrives (Added vs Removed) for the same (entity, componentType), +/// the old entry is removed so observers see the full state transition. /// internal class ChangeSet { private readonly List _changes = new(); - private readonly HashSet<(Entity Entity, ChangeKind Kind, Type? ComponentType)> _dedup = new(); + private readonly Dictionary<(Entity Entity, ChangeKind Kind, Type? ComponentType), int> _dedup = new(); /// /// All accumulated changes in insertion order. @@ -75,9 +76,31 @@ internal class ChangeSet private void TryAdd(EntityChange change) { var key = (change.Entity, change.Kind, change.ComponentType); - if (_dedup.Add(key)) + if (_dedup.ContainsKey(key)) { - _changes.Add(change); + // Same (entity, kind, componentType) already recorded — deduplicate. + return; } + + // When a component is re-added after being removed within the same + // batch, remove the old ComponentRemoved entry so observers see the + // full Add → Remove → Add sequence. + if (change.Kind == ChangeKind.ComponentAdded && change.ComponentType != null) + { + var removedKey = (change.Entity, ChangeKind.ComponentRemoved, change.ComponentType); + if (_dedup.Remove(removedKey, out int oldIndex)) + { + _changes.RemoveAt(oldIndex); + // Adjust indices for all entries that shifted down. + foreach (var k in _dedup.Keys.ToList()) + { + if (_dedup[k] > oldIndex) + _dedup[k]--; + } + } + } + + _dedup[key] = _changes.Count; + _changes.Add(change); } } diff --git a/src/OECS/QueryExecutor.cs b/src/OECS/QueryExecutor.cs index 40f235e..e8cb3a4 100644 --- a/src/OECS/QueryExecutor.cs +++ b/src/OECS/QueryExecutor.cs @@ -129,19 +129,95 @@ internal static class QueryExecutor var set3 = store.GetSet(typeof(T3)) as SparseSet; if (set3 == null) return; - var dense = set1.Dense; - var denseEntities = set1.DenseEntities; - var count = set1.Count; + // Pick the smallest set as the driver. + int c1 = set1.Count, c2 = set2.Count, c3 = set3.Count; + + if (c2 <= c1 && c2 <= c3) + { + var dense = set2.Dense; + var denseEntities = set2.DenseEntities; + var count = set2.Count; + for (int i = 0; i < count; i++) + { + var entity = denseEntities[i]; + if (entity.Id == SingletonId) continue; + if (!set1.Contains(entity)) continue; + if (!set3.Contains(entity)) continue; + if (!PassesWithoutFilter(store, entity, query.Without)) continue; + action(entity, ref set1.Get(entity), ref dense[i], ref set3.Get(entity)); + } + } + else if (c3 <= c1 && c3 <= c2) + { + var dense = set3.Dense; + var denseEntities = set3.DenseEntities; + var count = set3.Count; + for (int i = 0; i < count; i++) + { + var entity = denseEntities[i]; + if (entity.Id == SingletonId) continue; + if (!set1.Contains(entity)) continue; + if (!set2.Contains(entity)) continue; + if (!PassesWithoutFilter(store, entity, query.Without)) continue; + action(entity, ref set1.Get(entity), ref set2.Get(entity), ref dense[i]); + } + } + else + { + var dense = set1.Dense; + var denseEntities = set1.DenseEntities; + var count = set1.Count; + for (int i = 0; i < count; i++) + { + var entity = denseEntities[i]; + if (entity.Id == SingletonId) continue; + if (!set2.Contains(entity)) continue; + if (!set3.Contains(entity)) continue; + if (!PassesWithoutFilter(store, entity, query.Without)) continue; + action(entity, ref dense[i], ref set2.Get(entity), ref set3.Get(entity)); + } + } + } + + private static void IterateThree( + SparseSet driveSet, + SparseSet setA, + SparseSet setB, + ComponentStore store, + IReadOnlySet withoutTypes, + ForEachAction actionWhenDriverIsT2, + ForEachAction actionWhenDriverIsT1, + ForEachAction actionWhenDriverIsT3) + where TDriver : struct where TA : struct where TB : struct + { + var dense = driveSet.Dense; + var denseEntities = driveSet.DenseEntities; + var count = driveSet.Count; for (int i = 0; i < count; i++) { var entity = denseEntities[i]; if (entity.Id == SingletonId) continue; - if (!set2.Contains(entity)) continue; - if (!set3.Contains(entity)) continue; - if (!PassesWithoutFilter(store, entity, query.Without)) + if (!setA.Contains(entity)) continue; + if (!setB.Contains(entity)) continue; + if (!PassesWithoutFilter(store, entity, withoutTypes)) continue; - action(entity, ref dense[i], ref set2.Get(entity), ref set3.Get(entity)); + + if (actionWhenDriverIsT2 != null!) + { + // Driver is T2, so order is: T1=setA, T2=driveSet, T3=setB + actionWhenDriverIsT2(entity, ref setA.Get(entity), ref dense[i], ref setB.Get(entity)); + } + else if (actionWhenDriverIsT3 != null!) + { + // Driver is T3, so order is: T1=setA, T2=setB, T3=driveSet + actionWhenDriverIsT3(entity, ref setA.Get(entity), ref setB.Get(entity), ref dense[i]); + } + else + { + // Driver is T1, so order is: T1=driveSet, T2=setA, T3=setB + actionWhenDriverIsT1(entity, ref dense[i], ref setA.Get(entity), ref setB.Get(entity)); + } } } @@ -160,20 +236,73 @@ internal static class QueryExecutor var set4 = store.GetSet(typeof(T4)) as SparseSet; if (set4 == null) return; - var dense = set1.Dense; - var denseEntities = set1.DenseEntities; - var count = set1.Count; + // Pick the smallest set as the driver. + int c1 = set1.Count, c2 = set2.Count, c3 = set3.Count, c4 = set4.Count; + int min = Math.Min(Math.Min(c1, c2), Math.Min(c3, c4)); - for (int i = 0; i < count; i++) + if (min == c2) { - var entity = denseEntities[i]; - if (entity.Id == SingletonId) continue; - if (!set2.Contains(entity)) continue; - if (!set3.Contains(entity)) continue; - if (!set4.Contains(entity)) continue; - if (!PassesWithoutFilter(store, entity, query.Without)) - continue; - action(entity, ref dense[i], ref set2.Get(entity), ref set3.Get(entity), ref set4.Get(entity)); + var dense = set2.Dense; + var denseEntities = set2.DenseEntities; + var count = set2.Count; + for (int i = 0; i < count; i++) + { + var entity = denseEntities[i]; + if (entity.Id == SingletonId) continue; + if (!set1.Contains(entity)) continue; + if (!set3.Contains(entity)) continue; + if (!set4.Contains(entity)) continue; + if (!PassesWithoutFilter(store, entity, query.Without)) continue; + action(entity, ref set1.Get(entity), ref dense[i], ref set3.Get(entity), ref set4.Get(entity)); + } + } + else if (min == c3) + { + var dense = set3.Dense; + var denseEntities = set3.DenseEntities; + var count = set3.Count; + for (int i = 0; i < count; i++) + { + var entity = denseEntities[i]; + if (entity.Id == SingletonId) continue; + if (!set1.Contains(entity)) continue; + if (!set2.Contains(entity)) continue; + if (!set4.Contains(entity)) continue; + if (!PassesWithoutFilter(store, entity, query.Without)) continue; + action(entity, ref set1.Get(entity), ref set2.Get(entity), ref dense[i], ref set4.Get(entity)); + } + } + else if (min == c4) + { + var dense = set4.Dense; + var denseEntities = set4.DenseEntities; + var count = set4.Count; + for (int i = 0; i < count; i++) + { + var entity = denseEntities[i]; + if (entity.Id == SingletonId) continue; + if (!set1.Contains(entity)) continue; + if (!set2.Contains(entity)) continue; + if (!set3.Contains(entity)) continue; + if (!PassesWithoutFilter(store, entity, query.Without)) continue; + action(entity, ref set1.Get(entity), ref set2.Get(entity), ref set3.Get(entity), ref dense[i]); + } + } + else + { + var dense = set1.Dense; + var denseEntities = set1.DenseEntities; + var count = set1.Count; + for (int i = 0; i < count; i++) + { + var entity = denseEntities[i]; + if (entity.Id == SingletonId) continue; + if (!set2.Contains(entity)) continue; + if (!set3.Contains(entity)) continue; + if (!set4.Contains(entity)) continue; + if (!PassesWithoutFilter(store, entity, query.Without)) continue; + action(entity, ref dense[i], ref set2.Get(entity), ref set3.Get(entity), ref set4.Get(entity)); + } } } @@ -194,21 +323,94 @@ internal static class QueryExecutor var set5 = store.GetSet(typeof(T5)) as SparseSet; if (set5 == null) return; - var dense = set1.Dense; - var denseEntities = set1.DenseEntities; - var count = set1.Count; + // Pick the smallest set as the driver. + int c1 = set1.Count, c2 = set2.Count, c3 = set3.Count, c4 = set4.Count, c5 = set5.Count; + int min = Math.Min(Math.Min(Math.Min(c1, c2), Math.Min(c3, c4)), c5); - for (int i = 0; i < count; i++) + if (min == c2) { - var entity = denseEntities[i]; - if (entity.Id == SingletonId) continue; - if (!set2.Contains(entity)) continue; - if (!set3.Contains(entity)) continue; - if (!set4.Contains(entity)) continue; - if (!set5.Contains(entity)) continue; - if (!PassesWithoutFilter(store, entity, query.Without)) - continue; - action(entity, ref dense[i], ref set2.Get(entity), ref set3.Get(entity), ref set4.Get(entity), ref set5.Get(entity)); + var dense = set2.Dense; + var denseEntities = set2.DenseEntities; + var count = set2.Count; + for (int i = 0; i < count; i++) + { + var entity = denseEntities[i]; + if (entity.Id == SingletonId) continue; + if (!set1.Contains(entity)) continue; + if (!set3.Contains(entity)) continue; + if (!set4.Contains(entity)) continue; + if (!set5.Contains(entity)) continue; + if (!PassesWithoutFilter(store, entity, query.Without)) continue; + action(entity, ref set1.Get(entity), ref dense[i], ref set3.Get(entity), ref set4.Get(entity), ref set5.Get(entity)); + } + } + else if (min == c3) + { + var dense = set3.Dense; + var denseEntities = set3.DenseEntities; + var count = set3.Count; + for (int i = 0; i < count; i++) + { + var entity = denseEntities[i]; + if (entity.Id == SingletonId) continue; + if (!set1.Contains(entity)) continue; + if (!set2.Contains(entity)) continue; + if (!set4.Contains(entity)) continue; + if (!set5.Contains(entity)) continue; + if (!PassesWithoutFilter(store, entity, query.Without)) continue; + action(entity, ref set1.Get(entity), ref set2.Get(entity), ref dense[i], ref set4.Get(entity), ref set5.Get(entity)); + } + } + else if (min == c4) + { + var dense = set4.Dense; + var denseEntities = set4.DenseEntities; + var count = set4.Count; + for (int i = 0; i < count; i++) + { + var entity = denseEntities[i]; + if (entity.Id == SingletonId) continue; + if (!set1.Contains(entity)) continue; + if (!set2.Contains(entity)) continue; + if (!set3.Contains(entity)) continue; + if (!set5.Contains(entity)) continue; + if (!PassesWithoutFilter(store, entity, query.Without)) continue; + action(entity, ref set1.Get(entity), ref set2.Get(entity), ref set3.Get(entity), ref dense[i], ref set5.Get(entity)); + } + } + else if (min == c5) + { + var dense = set5.Dense; + var denseEntities = set5.DenseEntities; + var count = set5.Count; + for (int i = 0; i < count; i++) + { + var entity = denseEntities[i]; + if (entity.Id == SingletonId) continue; + if (!set1.Contains(entity)) continue; + if (!set2.Contains(entity)) continue; + if (!set3.Contains(entity)) continue; + if (!set4.Contains(entity)) continue; + if (!PassesWithoutFilter(store, entity, query.Without)) continue; + action(entity, ref set1.Get(entity), ref set2.Get(entity), ref set3.Get(entity), ref set4.Get(entity), ref dense[i]); + } + } + else + { + var dense = set1.Dense; + var denseEntities = set1.DenseEntities; + var count = set1.Count; + for (int i = 0; i < count; i++) + { + var entity = denseEntities[i]; + if (entity.Id == SingletonId) continue; + if (!set2.Contains(entity)) continue; + if (!set3.Contains(entity)) continue; + if (!set4.Contains(entity)) continue; + if (!set5.Contains(entity)) continue; + if (!PassesWithoutFilter(store, entity, query.Without)) continue; + action(entity, ref dense[i], ref set2.Get(entity), ref set3.Get(entity), ref set4.Get(entity), ref set5.Get(entity)); + } } } @@ -231,22 +433,117 @@ internal static class QueryExecutor var set6 = store.GetSet(typeof(T6)) as SparseSet; if (set6 == null) return; - var dense = set1.Dense; - var denseEntities = set1.DenseEntities; - var count = set1.Count; + // Pick the smallest set as the driver. + int c1 = set1.Count, c2 = set2.Count, c3 = set3.Count, c4 = set4.Count, c5 = set5.Count, c6 = set6.Count; + int min = Math.Min(Math.Min(Math.Min(c1, c2), Math.Min(c3, c4)), Math.Min(c5, c6)); - for (int i = 0; i < count; i++) + if (min == c2) { - var entity = denseEntities[i]; - if (entity.Id == SingletonId) continue; - if (!set2.Contains(entity)) continue; - if (!set3.Contains(entity)) continue; - if (!set4.Contains(entity)) continue; - if (!set5.Contains(entity)) continue; - if (!set6.Contains(entity)) continue; - if (!PassesWithoutFilter(store, entity, query.Without)) - continue; - action(entity, ref dense[i], ref set2.Get(entity), ref set3.Get(entity), ref set4.Get(entity), ref set5.Get(entity), ref set6.Get(entity)); + var dense = set2.Dense; + var denseEntities = set2.DenseEntities; + var count = set2.Count; + for (int i = 0; i < count; i++) + { + var entity = denseEntities[i]; + if (entity.Id == SingletonId) continue; + if (!set1.Contains(entity)) continue; + if (!set3.Contains(entity)) continue; + if (!set4.Contains(entity)) continue; + if (!set5.Contains(entity)) continue; + if (!set6.Contains(entity)) continue; + if (!PassesWithoutFilter(store, entity, query.Without)) continue; + action(entity, ref set1.Get(entity), ref dense[i], ref set3.Get(entity), ref set4.Get(entity), ref set5.Get(entity), ref set6.Get(entity)); + } + } + else if (min == c3) + { + var dense = set3.Dense; + var denseEntities = set3.DenseEntities; + var count = set3.Count; + for (int i = 0; i < count; i++) + { + var entity = denseEntities[i]; + if (entity.Id == SingletonId) continue; + if (!set1.Contains(entity)) continue; + if (!set2.Contains(entity)) continue; + if (!set4.Contains(entity)) continue; + if (!set5.Contains(entity)) continue; + if (!set6.Contains(entity)) continue; + if (!PassesWithoutFilter(store, entity, query.Without)) continue; + action(entity, ref set1.Get(entity), ref set2.Get(entity), ref dense[i], ref set4.Get(entity), ref set5.Get(entity), ref set6.Get(entity)); + } + } + else if (min == c4) + { + var dense = set4.Dense; + var denseEntities = set4.DenseEntities; + var count = set4.Count; + for (int i = 0; i < count; i++) + { + var entity = denseEntities[i]; + if (entity.Id == SingletonId) continue; + if (!set1.Contains(entity)) continue; + if (!set2.Contains(entity)) continue; + if (!set3.Contains(entity)) continue; + if (!set5.Contains(entity)) continue; + if (!set6.Contains(entity)) continue; + if (!PassesWithoutFilter(store, entity, query.Without)) continue; + action(entity, ref set1.Get(entity), ref set2.Get(entity), ref set3.Get(entity), ref dense[i], ref set5.Get(entity), ref set6.Get(entity)); + } + } + else if (min == c5) + { + var dense = set5.Dense; + var denseEntities = set5.DenseEntities; + var count = set5.Count; + for (int i = 0; i < count; i++) + { + var entity = denseEntities[i]; + if (entity.Id == SingletonId) continue; + if (!set1.Contains(entity)) continue; + if (!set2.Contains(entity)) continue; + if (!set3.Contains(entity)) continue; + if (!set4.Contains(entity)) continue; + if (!set6.Contains(entity)) continue; + if (!PassesWithoutFilter(store, entity, query.Without)) continue; + action(entity, ref set1.Get(entity), ref set2.Get(entity), ref set3.Get(entity), ref set4.Get(entity), ref dense[i], ref set6.Get(entity)); + } + } + else if (min == c6) + { + var dense = set6.Dense; + var denseEntities = set6.DenseEntities; + var count = set6.Count; + for (int i = 0; i < count; i++) + { + var entity = denseEntities[i]; + if (entity.Id == SingletonId) continue; + if (!set1.Contains(entity)) continue; + if (!set2.Contains(entity)) continue; + if (!set3.Contains(entity)) continue; + if (!set4.Contains(entity)) continue; + if (!set5.Contains(entity)) continue; + if (!PassesWithoutFilter(store, entity, query.Without)) continue; + action(entity, ref set1.Get(entity), ref set2.Get(entity), ref set3.Get(entity), ref set4.Get(entity), ref set5.Get(entity), ref dense[i]); + } + } + else + { + var dense = set1.Dense; + var denseEntities = set1.DenseEntities; + var count = set1.Count; + for (int i = 0; i < count; i++) + { + var entity = denseEntities[i]; + if (entity.Id == SingletonId) continue; + if (!set2.Contains(entity)) continue; + if (!set3.Contains(entity)) continue; + if (!set4.Contains(entity)) continue; + if (!set5.Contains(entity)) continue; + if (!set6.Contains(entity)) continue; + if (!PassesWithoutFilter(store, entity, query.Without)) continue; + action(entity, ref dense[i], ref set2.Get(entity), ref set3.Get(entity), ref set4.Get(entity), ref set5.Get(entity), ref set6.Get(entity)); + } } } } \ No newline at end of file diff --git a/src/OECS/World.cs b/src/OECS/World.cs index fe5e31b..cadf8bb 100644 --- a/src/OECS/World.cs +++ b/src/OECS/World.cs @@ -188,6 +188,17 @@ public class World : IDisposable // If replacing an existing relationship, remove the old index entry first. if (component is IRelationship newRel) { + // Guard against mismatched Source: the relationship must be stored + // on the entity it claims as its Source, otherwise the reverse + // index becomes corrupted. + if (newRel.Source != entity) + { + throw new InvalidOperationException( + $"Relationship of type {typeof(T).Name} has Source={newRel.Source} " + + $"but is being added to entity {entity}. " + + $"The Source must match the entity the component is added to."); + } + if (_components.TryGet(entity, out var old)) { var oldRel = (IRelationship)(object)old;