tokens: eliminate commit-path cache barriers, struct copying, and unguarded log allocations by SurbhiAgarwal1 · Pull Request #2364 · LFDT-Panurus/panurus

SurbhiAgarwal1 · 2026-09-13T16:59:28Z

Summary

Optimizes performance and memory allocations on the transaction commit path in token/services/tokens and token/services/utils/cache:

Cache Barrier Removal (ristretto.go): Removed synchronous c.cache.Wait() calls from write paths (Add/Delete/Clear), unlocking asynchronous Ristretto cache throughput. Exposed explicit Wait() helper for testing.
Pointer Pass Optimization (storage.go & tokens.go): Changed AppendToken parameter to pointer (*TokenToAppend) and updated CacheEntry.ToAppend, getActions, extractActions, and Parse to pass []*TokenToAppend, eliminating 13-field struct copying (~200+ bytes per output).
Event Buffer Capacity Retention (storage.go): Reset event buffer via t.pending = t.pending[:0] in FlushEvents and Rollback to retain allocated backing array capacity across transactions.
Upfront Slice Preallocation (tokens.go): Preallocated capacities for toSpend, toAppend, and toDelete slices.
Stack Trace Overhead Elimination (tokens.go): Guarded debug.Stack() in Service.DeleteTokens behind logger.IsEnabledFor(zapcore.DebugLevel).
Zero-Allocation Variadic Logging (tokens.go & storage.go): Wrapped hot-path DebugfContext log calls with if logger.IsEnabledFor(zapcore.DebugLevel) to eliminate Go variadic []any heap boxing allocations when debug logging is disabled.

Fixes #2188

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

AkramBitar

Review of 541179e8c..2d0ca3c53 (the localSession race fix + the tokens commit-path optimizations).

Verified before commenting: go build, go vet and go test are clean on all touched packages, plus -race -count=200 on the new session tests. I could not reproduce a close of closed channel / send on closed channel panic: writeChannel is closed only when closed && inFlight == 0, and inFlight is incremented under the same lock that rejects new sends, so that window is properly closed. Left and right also own disjoint write channels, so cross-side double-close is impossible.

The diff is largely sound. Six findings inline — one medium, the rest low. The medium (session.go Close() asymmetry) is worth addressing before merge; it leaks a goroutine in the same failure class #2285 set out to fix.

AkramBitar · 2026-09-30T10:02:53Z

Hello @SurbhiAgarwal1 ,

Any update with this PR?

Regards,
Akram

SurbhiAgarwal1 · 2026-09-30T13:50:19Z

Hi @AkramBitar,

Apologies for the delay!

All 6 review findings have been addressed in commit a39395e:

Unblock peer in send() on Close(): Added peerClosedChan so closing one side immediately releases any peer goroutine blocked in send().
Prevent silent message drops: send() now checks if the peer has closed and rejects the message with an error instead of silently buffering it into an abandoned channel.
Deterministic send(): Added a non-blocking attempt before selecting on ctx.Done() to eliminate the race with an already-ready buffered channel.
Read-your-write cache consistency: Restored synchronous Wait() calls in Add, Delete, and Clear in the Ristretto cache wrapper.
Event buffer aliasing: Reset pending = nil in FlushEvents and Rollback to eliminate backing-array aliasing during synchronous event publishing.
Caller attribution: Updated DeleteTokens to record the caller location via runtime.Caller(1) for consistent and accurate attribution.

The PR is ready for your review and approval!

Effi-S

Hi @SurbhiAgarwal1,
Thanks for the work here.
Please squash your commits to 1 and rebase to main

Effi-S

Some low findings:

Effi-S · 2026-10-01T13:40:35Z


 	// Step 3: Update tracked keys for next cycle
 	f.prevKeys = newKeys
+	f.cache.Wait()


Low: f.cache.Wait() runs after a loop of Add/Delete that each already Wait() internally.

Effi-S · 2026-10-01T13:43:45Z

+
+	// Goroutine 1: Continually calling Info() on left
+	wg.Go(func() {
+		for range 100 {


Nit: Duplicated/contradictory doc comment lines for TestLocalBidirectionalChannel_CloseReleasesPeerReceive (first line is truncated and unfinished).

Optimize performance and memory allocations on the transaction commit path in token/services/tokens and token/services/utils/cache, and resolve a data race in localSession under token/services/ttx: Commit-path optimizations: - Pass TokenToAppend by pointer (*TokenToAppend) to eliminate struct copying (~200+ bytes per output). - Retain event buffer slice capacity across transactions via reset in FlushEvents and Rollback. - Preallocate slice capacities for toSpend, toAppend, and toDelete. - Guard debug.Stack() and record caller location via runtime.Caller(1) in DeleteTokens. - Wrap hot-path DebugfContext calls with logger level checks to eliminate variadic heap allocations. - Expose explicit Wait() helper on Ristretto cache wrapper. localSession synchronization and unblocking: - Fix data race on localSession Closed state under concurrent self-auditing flows. - Unblock peer goroutines blocked in send() and receive() upon session Close(). - Prevent silent message drops by rejecting sends after peer session is closed. - Ensure deterministic non-blocking send prior to context cancellation check. - Add comprehensive race and session lifecycle tests. Fixes LFDT-Panurus#2188 Fixes LFDT-Panurus#2285 Signed-off-by: Surbhi Agarwal <SurbhiAgarwal1@users.noreply.github.com>

SurbhiAgarwal1 · 2026-10-02T09:56:56Z

Thanks for the review, @Effi-S!

All review comments have been addressed:

Removed the redundant f.cache.Wait() barrier and Wait() from the tokenCache interface in fetcher.go.
Added if logger.IsEnabledFor(zapcore.DebugLevel) guards around all remaining DebugfContext calls in tokens.go (AppendValid, CacheRequest, getActions, extractActions) and storage.go.
Removed the truncated duplicate doc comment in session_test.go.
Squashed into a single commit with DCO sign-off and rebased onto the latest main.

Effi-S

@SurbhiAgarwal1, thanks for your work here.
Some minor findings:

Effi-S · 2026-10-05T07:11:18Z

+		s.inFlight--
+		if s.inFlight == 0 && s.closed && !s.writeClosed {
+			s.writeClosed = true
+			close(s.writeChannel)


Medium:
Concurrent Send()/Close(): the writeChannel <- msg arm buffers the message, but the inner select then sees closedChan/peerClosedChan closed and returns "session is closed". Caller aborts while the peer still receives the message , one-sided state. Affects both fast-path and blocking select.

Effi-S · 2026-10-05T07:13:38Z

 		return nil
 	}

 	return s.readChannel


Medium:
readChannel is now closed on peer close, so <-Receive() returns nil / for range ends. In-repo consumers handle nil, but external view.Session consumers looping without a nil check busy-spin.

Effi-S · 2026-10-05T07:14:57Z

+// DeleteTokens marks the tokens as spent in the database, attributed to the caller's location.
 func (t *Service) DeleteTokens(ctx context.Context, ids ...*token2.ID) (err error) {
-	return t.DeleteTokensBy(ctx, string(debug.Stack()), ids...)
+	deletedBy := "Service.DeleteTokens"


Low deletedBy records only the direct caller (deleteTokens), losing the originating call chain. Fallback label no longer identifies the requester.

Effi-S · 2026-10-05T07:17:14Z


+// Wait blocks until all buffered writes and deletes are applied to the cache.
+// Useful for testing and deterministic cache state assertions.
+func (c *ristrettoCache[T]) Wait() {


Nit: Add/Delete/Clear already call c.cache.Wait(). The new exported Wait() and the four c.Wait() test calls are no-ops.

Copilot AI lite review requested due to automatic review settings September 13, 2026 16:59

Copilot AI reviewed Sep 13, 2026

Effi-S assigned SurbhiAgarwal1 Sep 14, 2026

Effi-S added this to the Q3/26 milestone Sep 14, 2026

Effi-S added performance improvements storage benchmark cleanup services and removed improvements labels Sep 14, 2026

SurbhiAgarwal1 force-pushed the perf/2188-commit-path-optimizations branch from 07555dc to 2d0ca3c Compare September 14, 2026 08:17

Effi-S requested a review from AkramBitar September 16, 2026 07:01

AkramBitar reviewed Sep 17, 2026

View reviewed changes

Comment thread token/services/ttx/session.go

Comment thread token/services/utils/cache/ristretto.go

Comment thread token/services/tokens/storage.go Outdated

Comment thread token/services/ttx/session.go

Comment thread token/services/ttx/session.go

Comment thread token/services/tokens/tokens.go

SurbhiAgarwal1 force-pushed the perf/2188-commit-path-optimizations branch from 76ac39e to a914243 Compare September 21, 2026 14:05

AkramBitar force-pushed the perf/2188-commit-path-optimizations branch from a914243 to a39395e Compare September 22, 2026 15:06

Effi-S requested changes Sep 30, 2026

View reviewed changes

SurbhiAgarwal1 force-pushed the perf/2188-commit-path-optimizations branch from a39395e to 3fe6194 Compare September 30, 2026 18:19

SurbhiAgarwal1 requested a review from Effi-S September 30, 2026 18:20

Effi-S requested changes Oct 1, 2026

View reviewed changes

SurbhiAgarwal1 force-pushed the perf/2188-commit-path-optimizations branch from 3fe6194 to 4343c0c Compare October 2, 2026 09:55

Effi-S self-requested a review October 5, 2026 07:00

Effi-S requested changes Oct 5, 2026

View reviewed changes

Conversation

SurbhiAgarwal1 commented Sep 13, 2026

Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

AkramBitar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AkramBitar commented Sep 30, 2026

Uh oh!

SurbhiAgarwal1 commented Sep 30, 2026

Uh oh!

Effi-S left a comment

Choose a reason for hiding this comment

Uh oh!

Effi-S left a comment

Choose a reason for hiding this comment

Uh oh!

Effi-S Oct 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Effi-S Oct 1, 2026

Choose a reason for hiding this comment

Uh oh!

SurbhiAgarwal1 commented Oct 2, 2026

Uh oh!

Effi-S left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Effi-S Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

Effi-S Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

Effi-S Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

Effi-S Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Effi-S left a comment •

edited

Loading