perf(txpool): use RLock in GetByID to allow concurrent pool reads#36
Open
simonzg wants to merge 2 commits into
Open
perf(txpool): use RLock in GetByID to allow concurrent pool reads#36simonzg wants to merge 2 commits into
simonzg wants to merge 2 commits into
Conversation
…ncurrent reads GetByID was acquiring an exclusive write lock (sync.RWMutex.Lock) for a read-only map lookup. This prevented concurrent reads and serialized every pool lookup: Get, GetTxObj, and the Contains fast-path in add() all funnel through GetByID and were forced to run one-at-a-time. Change to RLock/RUnlock so parallel reads can proceed concurrently. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
txObjectMap.GetByIDis a read-only map lookup, but it was acquiring an exclusive write lock (sync.RWMutex.Lock). This prevented any concurrent reads:Get,GetTxObj, and theContainsfast-path inadd()all route throughGetByIDand were serialized one-at-a-time regardless of load.On a busy node receiving many transactions simultaneously, every lookup — including the duplicate-check at the top of
add()— blocked every other lookup. Thesync.RWMutexis specifically designed to allow multiple concurrent readers, but only ifRLockis used.What Changed
GetByID:m.lock.Lock()/m.lock.Unlock()→m.lock.RLock()/m.lock.RUnlock()One line change. No behaviour change —
txObjMapis a plain map and map reads are safe underRLockby Go's memory model.Expected Impact
Get,GetTxObj, pool duplicate checks) no longer serialize behind a write lock.Addmethod already uses a proper write lock and does its own duplicate check internally, so correctness is unaffected.