perf(runtime): halve accepted-placement allocations via pooled operations

Cutover slice C2: the dormant placement path's per-operation cost was the
recorded activation blocker for routing frame-frequency traffic through
the canonical SetPosition owner (1,880 B/op measured at 4B2, cap 2,048).
Root-cause removal, not a raised cap: the per-operation envelope is now
pooled (bounded 64, reset-at-rent, InPool double-retire guard, cleared on
session reset/dispose and surfaced as a diagnostic ownership count), the
two engine-callback closures became one cached delegate over an explicit
context stack, and the pending-projection head read no longer boxes the
sorted enumerator. Measured 2,032 -> 944 B/op; the regression gate
tightens to 1,536. The residual floor is documented at the gate: ~520 B
inside Core's PhysicsEngine.SetPosition (outside this slice's scope) and
~208 B of sorted-tree node per pending receipt.

Pooling demanded — and received — the full staleness-discipline rework:
every frame holding an operation across a reentrancy point now captures
its never-reissued token and revalidates via fresh lookup
(IsCurrentByToken / token-shaped CancelCore), because a recycled
instance reinstalled at the same key makes every reference-identity
check a tautology. All ~26 sites audited (15 remain reference-based with
per-site no-reentrancy proofs); CommitCanonical's post-callback reads
are hoisted stack locals mirroring retail's savedTransientState pattern
(handle_all_collisions bits, pseudo-C 283952), its bookkeeping writes
are token-gated, and the settle path stays deliberately identity-
agnostic because retail's SetPositionInternal runs its physical settle
unconditionally even for displaced operations.

Reviewed: retail-conformance PASS + architecture/adversarial PASS after
two fix rounds (the ground-edge recycle window, the pool's cross-reset
retention, the class-wide tautology, a self-found snapshot-reference
iteration hazard). Runtime 927/927; complete Release solution 10,722
passed / 4 intentional skips; budget test green at the tightened gate.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-02 07:43:11 +02:00
parent 6460596b56
commit 63c601ff4d
2 changed files with 1362 additions and 191 deletions

View file

@ -1,5 +1,6 @@
using System.Collections.Immutable;
using System.Numerics;
using System.Reflection;
using AcDream.Content;
using AcDream.Content.Pak;
using AcDream.Core.Net;
@ -288,10 +289,28 @@ public sealed class RuntimeSetPositionStateTests
}
long allocated = GC.GetAllocatedBytesForCurrentThread() - before;
// The dormant 4B1 owner still allocates its operation/projection
// envelope. Pin the measured Release ceiling so 4B2 cannot activate
// the route without making this cost explicit or reducing it.
Assert.InRange(allocated / iterations, 1L, 2_048L);
// C2: root-caused and fixed the dormant 4B1 owner's per-operation
// allocations instead of raising this cap. Measured exactly 944 B/op
// (stable across 5+ repeat runs), down from the pre-fix 2,032 B/op -
// pooling `Operation` instances (BeginAcceptedPlacementCore no
// longer allocates a fresh instance every accepted placement),
// caching the `PhysicsEngine.SetPosition` collision-report delegate
// instead of a per-call closure, and replacing the LINQ
// `_pendingProjection.First()` pattern (which boxed
// SortedDictionary's struct Enumerator through IEnumerable<T> every
// acknowledgement) with a non-boxing foreach. See
// docs/research/2026-07-31-canonical-set-position.md for the C2
// finding this closes and the accepted residual floor: ~520 B/op
// lives inside AcDream.Core's PhysicsEngine.SetPosition (transition
// init/inner solve/query-footprint materialization - shared physics
// infrastructure, out of this Runtime-only slice's scope) and
// ~208 B/op is SortedDictionary's inherent per-Add tree-node
// allocation for `_pendingProjection` (replacing that ordered
// structure to chase the last ~230 B/op was judged too invasive/
// risky for the remaining headroom under this cap). 1,536 keeps
// roughly 60% headroom over the measured value for JIT/environment
// variance without re-opening the door to unbounded per-op growth.
Assert.InRange(allocated / iterations, 1L, 1_536L);
}
[Fact]
@ -1081,6 +1100,243 @@ public sealed class RuntimeSetPositionStateTests
Assert.True(lifetime.Physics.SetPosition.CaptureOwnership().IsConverged);
}
/// <summary>
/// F1 regression: from within the ground-edge HitGround callback, drive
/// an explicit cancel-then-begin for the SAME entity. Because the
/// Operation pool is LIFO, this retires the outer, still-executing
/// operation and immediately rents that EXACT instance back out for the
/// new ("recycled") operation - a strictly more adversarial recycle than
/// ReentrantGroundEdgePlacementCannotBeCancelledByDisplacedOperation's
/// begin-only chain (there, nothing had retired the outer operation to
/// the pool by the time the reentrant Begin ran, so it always drew a
/// different or fresh instance). CommitCanonical must still feed the
/// ORIGINAL (pre-callback) PreviousContact/PreviousOnWalkable into
/// HandleSetPositionCollisionReports, not whatever the recycled instance
/// now holds for the unrelated new operation. This is observable: the
/// environment-collision report only fires at all when
/// (!previousOnWalkable &amp;&amp; body.OnWalkable) - the recycled
/// instance's PreviousOnWalkable (captured from the body's ALREADY-
/// landed post-transition state) would read true instead of the
/// original false, silently suppressing the report if the operation's
/// live (potentially repurposed) fields were read instead of hoisted
/// locals.
/// </summary>
[Fact]
public void ReentrantCancelThenBeginRecyclesInstanceButCollisionReportUsesPreCallbackValues()
{
PhysicsEngine engine = FlatEngine(SourceLandblock, 0f);
engine.TransitionCellCollisionTestHook =
(transition, phase, _, observed) =>
{
if (phase == TransitionCellCollisionPhase.Environment)
{
transition.CollisionInfo.SetContactPlane(
new Plane(Vector3.UnitZ, 0f),
SourceCell);
}
return observed;
};
using var lifetime = new RuntimeEntityObjectLifetime(engine);
RuntimeEntityRecord record = CreateRecord(lifetime, 0x70001046u, 1);
PhysicsBody body = AttachBody(
lifetime,
record,
SourceCell,
PhysicsStateFlags.Gravity | PhysicsStateFlags.ReportCollisions);
var collisionObserver = new CollisionReportObserver();
using IDisposable collisionSubscription = lifetime.Physics
.CollisionReports.Subscribe(collisionObserver);
RuntimeEntityPlacementToken recycled = default;
var remote = new ReentrantRemotePlacement(body)
{
CellId = SourceCell,
OnHitGround = () =>
{
lifetime.Physics.SetPosition.Cancel(
record,
publishWithdrawal: false);
recycled = lifetime.Physics.SetPosition.BeginAcceptedPlacement(
record,
record.PositionAuthorityVersion,
RuntimeSetPositionOperationKind.RemoteAuthoritative);
},
};
lifetime.Entities.SetRemoteMotion(record, remote);
_ = lifetime.Physics.SetPosition.Apply(
record,
record.PositionAuthorityVersion,
Command(Request(SourceCell, new Vector3(13f, 18f, 7f))));
Assert.True(recycled.IsValid);
Assert.True(body.OnWalkable);
RuntimeCollisionReport report = Assert.Single(collisionObserver.Reports);
Assert.Equal(RuntimeCollisionReportKind.EnvironmentCollision, report.Kind);
Assert.False(report.RecipientWasInContact);
}
/// <summary>
/// Round 3 pinned regression: the coordinator's precise reachability
/// trace for the Cancel-path recycle hazard. <c>Cancel(record, bool)</c>
/// creates a withdrawal operation and installs it at this entity's key,
/// then calls <c>PublishPlacement(cancelledOld)</c> to notify observers
/// that the still-pending Place projection has been superseded by a
/// Discard - a synchronous dispatch that can reach an arbitrary
/// <see cref="IRuntimePlacementObserver"/> subscriber, which here
/// reentrantly calls <c>BeginAcceptedPlacement</c> for the SAME entity.
/// Because the Operation pool is LIFO and
/// <c>BeginAcceptedPlacementCore</c> rents only after retiring, that
/// reentrant Begin retires the withdrawal operation <c>Cancel</c> is
/// mid-way through publishing for and rents the exact same physical
/// instance right back out for a brand-new "inner" operation. Before
/// round 3's captured-token-vs-fresh-lookup conversion, <c>Cancel</c>'s
/// post-publish check read <c>IsCurrent(operation)</c> off that live
/// (now-repurposed) reference - a tautology that would have let the
/// outer withdrawal proceed to publish a SECOND (Withdraw) projection
/// stamped with the inner operation's state, corrupting a Begin-only
/// operation that never asked for a pending projection. The fix
/// captures <c>operation.Token</c> into a local BEFORE
/// <c>PublishPlacement</c> runs and re-verifies it via a fresh
/// <c>IsCurrentByToken</c> lookup afterward, so the stale reference is
/// detected and the outer call returns without publishing anything
/// beyond the Discard.
/// </summary>
[Fact]
public void ReentrantBeginDuringCancelPublishCannotBeOverwrittenByOuterWithdraw()
{
PhysicsEngine engine = FlatEngine(SourceLandblock, 0f);
using var lifetime = new RuntimeEntityObjectLifetime(engine);
RuntimeEntityRecord record = CreateRecord(lifetime, 0x70001250u, 1);
_ = AttachBody(lifetime, record, SourceCell);
RuntimeSetPositionOutcome pending = lifetime.Physics.SetPosition.Apply(
record,
record.PositionAuthorityVersion,
Command(Request(SourceCell, new Vector3(18f, 18f, 7f))));
Assert.Equal(
RuntimeSetPositionStatus.CommittedHostAcknowledgementPending,
pending.Status);
RuntimeEntityPlacementToken inner = default;
var observer = new PlacementObserver(delta =>
{
if (delta.Placement.Kind is RuntimePlacementProjectionKind.Discard)
{
inner = lifetime.Physics.SetPosition.BeginAcceptedPlacement(
record,
record.PositionAuthorityVersion,
RuntimeSetPositionOperationKind.RemoteAuthoritative);
}
});
using IDisposable subscription = lifetime.Events.SubscribePlacement(observer);
bool removed = lifetime.Physics.SetPosition.Cancel(
record,
publishWithdrawal: true);
Assert.True(removed);
Assert.True(inner.IsValid);
// Only the Discard (for the pending Place this Cancel superseded)
// should have published - a stale-reference bug would have added a
// second (Withdraw) delta stamped with `inner`'s state.
RuntimePlacementDelta delta = Assert.Single(observer.Deltas);
Assert.Equal(RuntimePlacementProjectionKind.Discard, delta.Placement.Kind);
// `inner` must still be exactly what BeginAcceptedPlacement handed
// back - untouched by the outer Cancel's withdrawal publish.
Assert.True(lifetime.Physics.SetPosition.IsPlacementCurrent(inner));
RuntimeSetPositionOwnershipSnapshot ownership =
lifetime.Physics.SetPosition.CaptureOwnership();
Assert.Equal(1, ownership.ActiveOperationCount);
Assert.Equal(1, ownership.AwaitingPreparationCount);
}
/// <summary>
/// Round 3 CancelCore-shape regression: from within the ground-edge
/// HitGround callback, recycle the SAME operation instance
/// (cancel-then-begin, LIFO pool) for a brand-new "inner" operation on
/// the same entity, AND advance the record's PlacementCommitVersion a
/// second time (exactly what a completed nested commit would also have
/// produced) so CommitCanonical's post-callback
/// IsCanonicalPlacementCommitCurrent check fails for the OUTER commit.
/// SubmitPreparedPlacementCore's own failure branch then calls
/// <c>PublishCancellation(CancelCore(token.Entity, token))</c> using
/// `token` - the outer caller's own (now-stale) captured token. Before
/// round 3, <c>CancelCore(Operation expected, ...)</c> compared
/// `ReferenceEquals(current, expected)`; with the physical instance
/// recycled for `inner`, that check would have found the SAME instance
/// at this key and retired/cancelled it out from under the still-active
/// inner operation - the "worse outcome: cancelling the newer
/// operation" the reviewer flagged. The Token-keyed
/// <c>CancelCore(key, expectedToken)</c> must instead see
/// `current.Token != expectedToken` and no-op, leaving `inner`
/// untouched.
/// </summary>
[Fact]
public void ReentrantCancelThenBeginDuringCommitFailureLeavesInnerOperationUncancelled()
{
PhysicsEngine engine = FlatEngine(SourceLandblock, 0f);
engine.TransitionCellCollisionTestHook =
(transition, phase, _, observed) =>
{
if (phase == TransitionCellCollisionPhase.Environment)
{
transition.CollisionInfo.SetContactPlane(
new Plane(Vector3.UnitZ, 0f),
SourceCell);
}
return observed;
};
using var lifetime = new RuntimeEntityObjectLifetime(engine);
RuntimeEntityRecord record = CreateRecord(lifetime, 0x70001251u, 1);
PhysicsBody body = AttachBody(
lifetime,
record,
SourceCell,
PhysicsStateFlags.Gravity | PhysicsStateFlags.ReportCollisions);
RuntimeEntityPlacementToken inner = default;
var remote = new ReentrantRemotePlacement(body)
{
CellId = SourceCell,
OnHitGround = () =>
{
lifetime.Physics.SetPosition.Cancel(
record,
publishWithdrawal: false);
// Simulate a nested commit completing for `inner` (without
// needing a full nested SetPosition round-trip): advance
// PlacementCommitVersion BEFORE `inner`'s operation is
// created so `inner` snapshots the already-advanced value
// and stays internally self-consistent, while the OUTER
// commit's `canonicalCommitVersion` (captured before this
// callback ran) now mismatches the record.
lifetime.Entities.AdvancePlacementCommit(record);
inner = lifetime.Physics.SetPosition.BeginAcceptedPlacement(
record,
record.PositionAuthorityVersion,
RuntimeSetPositionOperationKind.RemoteAuthoritative);
},
};
lifetime.Entities.SetRemoteMotion(record, remote);
RuntimeSetPositionOutcome outcome = lifetime.Physics.SetPosition.Apply(
record,
record.PositionAuthorityVersion,
Command(Request(SourceCell, new Vector3(13f, 18f, 7f))));
Assert.Equal(RuntimeSetPositionStatus.Cancelled, outcome.Status);
Assert.True(inner.IsValid);
// The bug this pins: a reference-based CancelCore would retire the
// recycled instance (now serving `inner`) out from under it.
// Confirm `inner` is still the live, current operation for this
// entity.
Assert.True(lifetime.Physics.SetPosition.IsPlacementCurrent(inner));
RuntimeSetPositionOwnershipSnapshot ownership =
lifetime.Physics.SetPosition.CaptureOwnership();
Assert.Equal(1, ownership.ActiveOperationCount);
Assert.Equal(1, ownership.AwaitingPreparationCount);
}
[Fact]
public void RetrySnapshotPreservesOrderWhenObserverAcknowledgesTwoPendingTokens()
{
@ -2012,6 +2268,158 @@ public sealed class RuntimeSetPositionStateTests
Assert.True(lifetime.Physics.CaptureOwnership().IsConverged);
}
/// <summary>
/// F2 regression: the C2 Operation pool retains full previous-generation
/// entity graphs (Record -&gt; Snapshot/PhysicsBody/clock/host references)
/// through every pooled instance until it is rented and reset - a
/// session reset must not leave that behind uncleared, even though
/// PooledOperationCount is deliberately excluded from
/// <see cref="RuntimeSetPositionOwnershipSnapshot.IsConverged"/> (see
/// its doc comment).
/// </summary>
[Fact]
public void OperationPoolClearsOnResetSession()
{
PhysicsEngine engine = FlatEngine(SourceLandblock, 0f);
using var lifetime = new RuntimeEntityObjectLifetime(engine);
RuntimeEntityRecord record = CreateRecord(lifetime, 0x70001048u, 1);
_ = AttachBody(lifetime, record, SourceCell);
RuntimeSetPositionOutcome outcome = lifetime.Physics.SetPosition.Apply(
record,
record.PositionAuthorityVersion,
Command(Request(SourceCell, new Vector3(11f, 18f, 7f))));
Assert.True(lifetime.Physics.SetPosition.AcknowledgeProjection(
outcome.Projection));
RuntimeSetPositionOwnershipSnapshot beforeReset =
lifetime.Physics.SetPosition.CaptureOwnership();
Assert.True(beforeReset.PooledOperationCount >= 1);
lifetime.Physics.SetPosition.ResetSession();
Assert.Equal(
0,
lifetime.Physics.SetPosition.CaptureOwnership().PooledOperationCount);
}
/// <summary>
/// F2 regression: same guarantee as
/// <see cref="OperationPoolClearsOnResetSession"/>, via Dispose instead
/// of ResetSession - both callers of ClearOwnedState must clear the
/// pool.
/// </summary>
[Fact]
public void OperationPoolClearsOnDispose()
{
PhysicsEngine engine = FlatEngine(SourceLandblock, 0f);
var lifetime = new RuntimeEntityObjectLifetime(engine);
RuntimeEntityRecord record = CreateRecord(lifetime, 0x70001049u, 1);
_ = AttachBody(lifetime, record, SourceCell);
RuntimeSetPositionOutcome outcome = lifetime.Physics.SetPosition.Apply(
record,
record.PositionAuthorityVersion,
Command(Request(SourceCell, new Vector3(11f, 18f, 7f))));
Assert.True(lifetime.Physics.SetPosition.AcknowledgeProjection(
outcome.Projection));
RuntimeSetPositionOwnershipSnapshot beforeDispose =
lifetime.Physics.SetPosition.CaptureOwnership();
Assert.True(beforeDispose.PooledOperationCount >= 1);
lifetime.Dispose();
Assert.Equal(
0,
lifetime.Physics.SetPosition.CaptureOwnership().PooledOperationCount);
}
/// <summary>
/// F4 regression: converting Operation's properties from
/// <c>required { get; init; }</c> to plain <c>{ get; set; }</c> (so
/// pooling could recycle instances) dropped the compiler's completeness
/// net - nothing any longer forces every construction site to set every
/// field. This reflection-based test is the runtime replacement: it
/// pins the exact set of backing fields Operation declares (private, so
/// only reflection can reach it from a test) against a hardcoded,
/// maintained list. Adding a new auto-property to Operation changes its
/// backing-field set and fails this test immediately - the failure
/// message is the prompt to update BOTH this list and
/// <c>Operation.ResetAllFieldsToDefault</c> in the same change, exactly
/// mirroring what a missing `required` member assignment used to force
/// at compile time.
/// </summary>
[Fact]
public void OperationResetAllFieldsToDefaultTouchesEveryDeclaredField()
{
Type? operationType = typeof(RuntimeSetPositionState).GetNestedType(
"Operation",
BindingFlags.NonPublic);
Assert.NotNull(operationType);
FieldInfo[] actualFields = operationType!.GetFields(
BindingFlags.Instance
| BindingFlags.NonPublic
| BindingFlags.Public);
string[] actualFieldNames = actualFields
.Select(field => field.Name)
.OrderBy(name => name, StringComparer.Ordinal)
.ToArray();
// The maintained list: one entry per auto-property Operation
// declares. Kept as property names (not the `<Name>k__BackingField`
// form the compiler actually emits) so a reviewer can read it
// directly against the property list in the source file.
string[] expectedPropertyNames =
[
"Record",
"Body",
"Token",
"Key",
"PositionAuthorityVersion",
"SessionLifetimeVersion",
"SourceSpatialAuthorityVersion",
"SourceVelocityAuthorityVersion",
"PreviousContact",
"PreviousOnWalkable",
"Command",
"Result",
"SpatialAuthorityVersion",
"PlacementCommitVersion",
"ExactCellId",
"CollisionGeneration",
"CollisionPrefix",
"WithdrawalAcknowledged",
"CollisionGenerationReady",
"CollisionQuiescenceHeld",
"ProjectionSequence",
"WakeableLostCell",
"Stage",
"Kind",
"Portal",
"RequiresPreparation",
"Expired",
"LostFamilyKeys",
"InheritedLostDeadline",
"EnteringWorldFromCelllessResidence",
"DormantLocalActivation",
"PreparedCommandAwaitingWithdrawalAck",
"InPool",
];
string[] expectedFieldNames = expectedPropertyNames
.Select(name => $"<{name}>k__BackingField")
.OrderBy(name => name, StringComparer.Ordinal)
.ToArray();
Assert.Equal(expectedFieldNames, actualFieldNames);
MethodInfo? resetMethod = operationType.GetMethod(
"ResetAllFieldsToDefault",
BindingFlags.Instance | BindingFlags.NonPublic);
Assert.NotNull(resetMethod);
}
[Fact]
public void CommittedParentDoesNotLeakAcrossChildGuidReuse()
{