diff --git a/docs/garbage-collector.md b/docs/garbage-collector.md index f641e74a..a6539eac 100644 --- a/docs/garbage-collector.md +++ b/docs/garbage-collector.md @@ -158,6 +158,8 @@ Two kinds of lock coordinate the collectors, with a strict order between them: - **The global collect lock** serializes `Collect`/`CollectYoung` across all collectors — the mark epoch (`GCCurrentMark`) is process-global and some intrinsic objects are shared, so two concurrent collections advancing the epoch mid-mark would unmark each other's live sets. - **A per-collector accounting lock** is a short-lived leaf lock for the byte counters a *foreign* thread can drive (`BytesAllocated`, the external-byte total, the forced-collect floor, the peak). The one cross-thread entry point into a collector is `ReleaseExternalBytes` through an error object's reserving-collector pointer (an error charged on collector A can be destroyed on worker thread B); everything else reaches a collector through the thread-local `Instance`, so the managed-object list and root sets are owner-thread-confined and unlocked. Keeping the counters on their own per-collector lock means one worker's full mark-and-sweep does not stall any other worker's per-allocation register/unregister — and workers never contend with each other on the allocation path at all, only with the rare cross-thread release aimed at their own collector. +Readers of those counters do not take the accounting lock on 64-bit targets: every write goes through the lock, and an aligned `Int64` load cannot tear there, so `BytesAllocated` stays a bare field read on the per-allocation path. On 32-bit targets a cross-thread `ReleaseExternalBytes` splits its write into two stores, so the `BytesAllocated` accessor takes the lock for the load — no 64-bit interlocked read is available, since FPC 3.2.2 declares those only under `CPU64` and CI also builds i386-win32. The peak and lifetime totals are written only by the owning thread and need neither treatment. + The order is collect lock → accounting lock, never the reverse (and never two accounting locks at once): a sweep settles its freed bytes — and a swept error destructor releases its cross-collector charge — under an accounting lock while the collect lock is held, and the reservation path drops the accounting lock before it collects. The accounting lock lives and dies with its collector; a cross-thread release entering it is safe on the same terms as the counter fields it guards, per the lifecycle invariant that a charged object is freed before its reserving collector shuts down. Key behavior on worker threads: diff --git a/source/units/Goccia.GarbageCollector.pas b/source/units/Goccia.GarbageCollector.pas index 29afca0b..81571354 100644 --- a/source/units/Goccia.GarbageCollector.pas +++ b/source/units/Goccia.GarbageCollector.pas @@ -163,6 +163,15 @@ TGarbageCollector = class // shared cache line entirely: each worker contends only with the rare // cross-thread release actually aimed at its collector. // + // Readers: every write to these counters goes through this lock, so a + // reader only has to avoid observing a half-applied cross-thread write. + // On 64-bit an aligned Int64 load cannot tear, so readers take nothing and + // BytesAllocated stays a bare field load on the per-allocation path; on + // 32-bit ReleaseExternalBytes splits its write into two stores and readers + // go through GetBytesAllocated, which takes this lock. Code that already + // holds the lock reads the fields directly — GetBytesAllocated must never + // be called from inside a section. + // // Lock order: the global GCCollectLock may be held when taking an // accounting lock (a sweep's aggregate byte settle, a swept destructor's // cross-collector release). NEVER take GCCollectLock — or a second @@ -210,8 +219,19 @@ TGarbageCollector = class function GetManagedObjectCount: Integer; function GetWatermark: Integer; {$IFDEF FPC}inline;{$ENDIF} + // Untorn read of the live-byte total for callers that hold no lock — see + // the reader rule at FAccountingLock. Never call it while holding the + // accounting lock; read FBytesAllocated directly there. + function GetBytesAllocated: Int64; {$IFDEF FPC}inline;{$ENDIF} + // Pressure predicate over an already-read live total, so the charge path + // can decide from the value it just committed instead of re-reading it + // through the locked accessor with the lock held. + function NeedsMemoryPressureCollection( + const ABytesAllocated: Int64): Boolean; overload; procedure ClearActiveRootEntries(const AObject: TGCManagedObject); procedure GrowActiveRootStack; + // Reads the live total and the forced-collect floor, so the caller must + // hold FAccountingLock across the decision it feeds. function ShouldForceLimitCollection(const ABytes: Int64): Boolean; // The one --max-memory fit predicate (overflow guard + ceiling test). // Caller must hold FAccountingLock ("Locked" suffix). @@ -274,7 +294,7 @@ TGarbageCollector = class // active root stack so a stack-held object survives the collection. procedure CollectIfNeeded(const AProtect: TGCManagedObject); overload; - function NeedsMemoryPressureCollection: Boolean; + function NeedsMemoryPressureCollection: Boolean; overload; // Collects when pressure has been latched by an external reservation or // when the live set has crossed the reserve below the ceiling. AForce @@ -336,7 +356,12 @@ TGarbageCollector = class // number of bytes currently tracked by the GC (InstanceSize per // registered object). Set MaxBytes to a positive value to impose // a ceiling; allocations that exceed it raise a RangeError. - property BytesAllocated: Int64 read FBytesAllocated; + // + // BytesAllocated reads through an accessor because it is the one counter a + // foreign thread writes (ReleaseExternalBytes). The peak and the lifetime + // total are written only by the owning thread, so no reader of theirs can + // catch a half-applied write and they stay direct field reads. + property BytesAllocated: Int64 read GetBytesAllocated; property PeakBytesAllocated: Int64 read FPeakBytesAllocated; property TotalBytesAllocated: Int64 read FTotalBytesAllocated; property MaxBytes: Int64 read FMaxBytes write FMaxBytes; @@ -1142,7 +1167,35 @@ procedure TGarbageCollector.CollectIfNeeded( end; end; +function TGarbageCollector.GetBytesAllocated: Int64; +begin + {$IF SizeOf(Pointer) >= 8} + Result := FBytesAllocated; + {$ELSE} + // A 32-bit target splits the 64-bit write in a cross-thread + // ReleaseExternalBytes into two stores, and a load that lands between them + // yields a total no memory-limit decision may be made on. No 64-bit + // interlocked load is available: FPC 3.2.2 declares those only under CPU64, + // and CI builds i386-win32. Nothing between the two calls can raise, so the + // load needs no exception frame — which also keeps this accessor inlinable. + CriticalSectionEnter(FAccountingLock); + Result := FBytesAllocated; + CriticalSectionLeave(FAccountingLock); + {$ENDIF} +end; + function TGarbageCollector.NeedsMemoryPressureCollection: Boolean; +begin + // Guards before the counter read: on 32-bit GetBytesAllocated takes the + // accounting lock, and the periodic VM/interpreter pressure polls must stay + // free when no limit is set or a collection is already underway. + if (FMaxBytes <= 0) or FCollecting or FMemoryLimitFiring then + Exit(False); + Result := NeedsMemoryPressureCollection(GetBytesAllocated); +end; + +function TGarbageCollector.NeedsMemoryPressureCollection( + const ABytesAllocated: Int64): Boolean; var Reserve: Int64; begin @@ -1160,7 +1213,7 @@ function TGarbageCollector.NeedsMemoryPressureCollection: Boolean; if Reserve >= FMaxBytes then Reserve := FMaxBytes div 2; - Result := FBytesAllocated >= (FMaxBytes - Reserve); + Result := ABytesAllocated >= (FMaxBytes - Reserve); end; procedure TGarbageCollector.CollectForMemoryPressure( @@ -1403,7 +1456,7 @@ function TGarbageCollector.TryChargeExternalBytesLocked( FPeakBytesAllocated := FBytesAllocated; if (FExternalBytesAllocatedSinceGC >= EXTERNAL_MEMORY_PRESSURE_ALLOCATION_INTERVAL) or - NeedsMemoryPressureCollection then + NeedsMemoryPressureCollection(FBytesAllocated) then begin FExternalPressurePending := True; if Assigned(FMemoryPressureCountdown) then diff --git a/source/units/Goccia.MemoryLimit.pas b/source/units/Goccia.MemoryLimit.pas index 6bc4e95d..c482e50f 100644 --- a/source/units/Goccia.MemoryLimit.pas +++ b/source/units/Goccia.MemoryLimit.pas @@ -130,6 +130,7 @@ constructor TGocciaMemoryLimitError.Create(const ARequestedBytes, function CanAllocateNativeBytes(const ABytes: Int64): Boolean; var GC: TGarbageCollector; + LiveBytes: Int64; begin if ABytes <= 0 then Exit(True); @@ -138,11 +139,15 @@ function CanAllocateNativeBytes(const ABytes: Int64): Boolean; one are unbounded by construction, and that is their choice to make. } if not Assigned(GC) or (GC.MaxBytes <= 0) then Exit(True); + { One snapshot for both tests: a cross-thread release between two reads + would split the decision across totals, and on 32-bit each read is a + locked load. } + LiveBytes := GC.BytesAllocated; { Overflow guard first: a JS-controlled length can multiply into a value that wraps, and a wrapped total would compare as comfortably in budget. } - if GC.BytesAllocated > High(Int64) - ABytes then + if LiveBytes > High(Int64) - ABytes then Exit(False); - Result := GC.BytesAllocated + ABytes <= GC.MaxBytes; + Result := LiveBytes + ABytes <= GC.MaxBytes; end; procedure RequireNativeBytes(const ABytes: Int64);