Skip to content

Commit 1b0168d

Browse files
committed
perf(pool): replace hookManager RWMutex with atomic.Pointer and add predefined state slices
- Replace hookManager RWMutex with atomic.Pointer for lock-free reads in hot paths - Add predefined state slices to avoid allocations (validFromInUse, validFromCreatedOrIdle, etc.) - Add Clone() method to PoolHookManager for atomic updates - Update AddPoolHook/RemovePoolHook to use copy-on-write pattern - Update all hookManager access points to use atomic Load() Performance improvements: - Eliminates RWMutex contention in Get/Put/Remove hot paths - Reduces allocations by reusing predefined state slices - Lock-free reads allow better CPU cache utilization
1 parent 23d0e0f commit 1b0168d

File tree

4 files changed

+59
-30
lines changed

4 files changed

+59
-30
lines changed

internal/pool/conn_state.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,14 @@ const (
4141
StateClosed
4242
)
4343

44+
// Predefined state slices to avoid allocations in hot paths
45+
var (
46+
validFromInUse = []ConnState{StateInUse}
47+
validFromCreatedOrIdle = []ConnState{StateCreated, StateIdle}
48+
validFromCreatedOrInUse = []ConnState{StateCreated, StateInUse}
49+
validFromCreatedInUseOrIdle = []ConnState{StateCreated, StateInUse, StateIdle}
50+
)
51+
4452
// String returns a human-readable string representation of the state.
4553
func (s ConnState) String() string {
4654
switch s {

internal/pool/hooks.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,3 +140,16 @@ func (phm *PoolHookManager) GetHooks() []PoolHook {
140140
copy(hooks, phm.hooks)
141141
return hooks
142142
}
143+
144+
// Clone creates a copy of the hook manager with the same hooks.
145+
// This is used for lock-free atomic updates of the hook manager.
146+
func (phm *PoolHookManager) Clone() *PoolHookManager {
147+
phm.hooksMu.RLock()
148+
defer phm.hooksMu.RUnlock()
149+
150+
newManager := &PoolHookManager{
151+
hooks: make([]PoolHook, len(phm.hooks)),
152+
}
153+
copy(newManager.hooks, phm.hooks)
154+
return newManager
155+
}

internal/pool/hooks_test.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -202,26 +202,29 @@ func TestPoolWithHooks(t *testing.T) {
202202
pool.AddPoolHook(testHook)
203203

204204
// Verify hooks are initialized
205-
if pool.hookManager == nil {
205+
manager := pool.hookManager.Load()
206+
if manager == nil {
206207
t.Error("Expected hookManager to be initialized")
207208
}
208209

209-
if pool.hookManager.GetHookCount() != 1 {
210-
t.Errorf("Expected 1 hook in pool, got %d", pool.hookManager.GetHookCount())
210+
if manager.GetHookCount() != 1 {
211+
t.Errorf("Expected 1 hook in pool, got %d", manager.GetHookCount())
211212
}
212213

213214
// Test adding hook to pool
214215
additionalHook := &TestHook{ShouldPool: true, ShouldAccept: true}
215216
pool.AddPoolHook(additionalHook)
216217

217-
if pool.hookManager.GetHookCount() != 2 {
218-
t.Errorf("Expected 2 hooks after adding, got %d", pool.hookManager.GetHookCount())
218+
manager = pool.hookManager.Load()
219+
if manager.GetHookCount() != 2 {
220+
t.Errorf("Expected 2 hooks after adding, got %d", manager.GetHookCount())
219221
}
220222

221223
// Test removing hook from pool
222224
pool.RemovePoolHook(additionalHook)
223225

224-
if pool.hookManager.GetHookCount() != 1 {
225-
t.Errorf("Expected 1 hook after removing, got %d", pool.hookManager.GetHookCount())
226+
manager = pool.hookManager.Load()
227+
if manager.GetHookCount() != 1 {
228+
t.Errorf("Expected 1 hook after removing, got %d", manager.GetHookCount())
226229
}
227230
}

internal/pool/pool.go

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -148,8 +148,8 @@ type ConnPool struct {
148148
_closed uint32 // atomic
149149

150150
// Pool hooks manager for flexible connection processing
151-
hookManagerMu sync.RWMutex
152-
hookManager *PoolHookManager
151+
// Using atomic.Pointer for lock-free reads in hot paths (Get/Put)
152+
hookManager atomic.Pointer[PoolHookManager]
153153
}
154154

155155
var _ Pooler = (*ConnPool)(nil)
@@ -176,27 +176,37 @@ func NewConnPool(opt *Options) *ConnPool {
176176

177177
// initializeHooks sets up the pool hooks system.
178178
func (p *ConnPool) initializeHooks() {
179-
p.hookManager = NewPoolHookManager()
179+
manager := NewPoolHookManager()
180+
p.hookManager.Store(manager)
180181
}
181182

182183
// AddPoolHook adds a pool hook to the pool.
183184
func (p *ConnPool) AddPoolHook(hook PoolHook) {
184-
p.hookManagerMu.Lock()
185-
defer p.hookManagerMu.Unlock()
186-
187-
if p.hookManager == nil {
185+
// Lock-free read of current manager
186+
manager := p.hookManager.Load()
187+
if manager == nil {
188188
p.initializeHooks()
189+
manager = p.hookManager.Load()
189190
}
190-
p.hookManager.AddHook(hook)
191+
192+
// Create new manager with added hook
193+
newManager := manager.Clone()
194+
newManager.AddHook(hook)
195+
196+
// Atomically swap to new manager
197+
p.hookManager.Store(newManager)
191198
}
192199

193200
// RemovePoolHook removes a pool hook from the pool.
194201
func (p *ConnPool) RemovePoolHook(hook PoolHook) {
195-
p.hookManagerMu.Lock()
196-
defer p.hookManagerMu.Unlock()
202+
manager := p.hookManager.Load()
203+
if manager != nil {
204+
// Create new manager with removed hook
205+
newManager := manager.Clone()
206+
newManager.RemoveHook(hook)
197207

198-
if p.hookManager != nil {
199-
p.hookManager.RemoveHook(hook)
208+
// Atomically swap to new manager
209+
p.hookManager.Store(newManager)
200210
}
201211
}
202212

@@ -444,11 +454,8 @@ func (p *ConnPool) getConn(ctx context.Context) (*Conn, error) {
444454
now := time.Now()
445455
attempts := 0
446456

447-
// Get hooks manager once for this getConn call for performance.
448-
// Note: Hooks added/removed during this call won't be reflected.
449-
p.hookManagerMu.RLock()
450-
hookManager := p.hookManager
451-
p.hookManagerMu.RUnlock()
457+
// Lock-free atomic read - no mutex overhead!
458+
hookManager := p.hookManager.Load()
452459

453460
for {
454461
if attempts >= getAttempts {
@@ -651,9 +658,8 @@ func (p *ConnPool) putConn(ctx context.Context, cn *Conn, freeTurn bool) {
651658
// It's a push notification, allow pooling (client will handle it)
652659
}
653660

654-
p.hookManagerMu.RLock()
655-
hookManager := p.hookManager
656-
p.hookManagerMu.RUnlock()
661+
// Lock-free atomic read - no mutex overhead!
662+
hookManager := p.hookManager.Load()
657663

658664
if hookManager != nil {
659665
shouldPool, shouldRemove, err = hookManager.ProcessOnPut(ctx, cn)
@@ -742,9 +748,8 @@ func (p *ConnPool) RemoveWithoutTurn(ctx context.Context, cn *Conn, reason error
742748

743749
// removeConnInternal is the internal implementation of Remove that optionally frees a turn.
744750
func (p *ConnPool) removeConnInternal(ctx context.Context, cn *Conn, reason error, freeTurn bool) {
745-
p.hookManagerMu.RLock()
746-
hookManager := p.hookManager
747-
p.hookManagerMu.RUnlock()
751+
// Lock-free atomic read - no mutex overhead!
752+
hookManager := p.hookManager.Load()
748753

749754
if hookManager != nil {
750755
hookManager.ProcessOnRemove(ctx, cn, reason)

0 commit comments

Comments
 (0)