Skip to content

Commit bc70b52

Browse files
authored
perf: avoid unnecessary copy operation (#3376)
* optime: reduce unnecessary copy operations * add a comment * trigger CI without code changes, because the bug of docker * add comments
1 parent ba26e35 commit bc70b52

File tree

1 file changed

+12
-9
lines changed

1 file changed

+12
-9
lines changed

ring.go

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -349,17 +349,16 @@ func (c *ringSharding) newRingShards(
349349
return
350350
}
351351

352+
// Warning: External exposure of `c.shards.list` may cause data races.
353+
// So keep internal or implement deep copy if exposed.
352354
func (c *ringSharding) List() []*ringShard {
353-
var list []*ringShard
354-
355355
c.mu.RLock()
356-
if !c.closed {
357-
list = make([]*ringShard, len(c.shards.list))
358-
copy(list, c.shards.list)
359-
}
360-
c.mu.RUnlock()
356+
defer c.mu.RUnlock()
361357

362-
return list
358+
if c.closed {
359+
return nil
360+
}
361+
return c.shards.list
363362
}
364363

365364
func (c *ringSharding) Hash(key string) string {
@@ -423,6 +422,7 @@ func (c *ringSharding) Heartbeat(ctx context.Context, frequency time.Duration) {
423422
case <-ticker.C:
424423
var rebalance bool
425424

425+
// note: `c.List()` return a shadow copy of `[]*ringShard`.
426426
for _, shard := range c.List() {
427427
err := shard.Client.Ping(ctx).Err()
428428
isUp := err == nil || err == pool.ErrPoolTimeout
@@ -582,6 +582,7 @@ func (c *Ring) retryBackoff(attempt int) time.Duration {
582582

583583
// PoolStats returns accumulated connection pool stats.
584584
func (c *Ring) PoolStats() *PoolStats {
585+
// note: `c.List()` return a shadow copy of `[]*ringShard`.
585586
shards := c.sharding.List()
586587
var acc PoolStats
587588
for _, shard := range shards {
@@ -651,6 +652,7 @@ func (c *Ring) ForEachShard(
651652
ctx context.Context,
652653
fn func(ctx context.Context, client *Client) error,
653654
) error {
655+
// note: `c.List()` return a shadow copy of `[]*ringShard`.
654656
shards := c.sharding.List()
655657
var wg sync.WaitGroup
656658
errCh := make(chan error, 1)
@@ -682,6 +684,7 @@ func (c *Ring) ForEachShard(
682684
}
683685

684686
func (c *Ring) cmdsInfo(ctx context.Context) (map[string]*CommandInfo, error) {
687+
// note: `c.List()` return a shadow copy of `[]*ringShard`.
685688
shards := c.sharding.List()
686689
var firstErr error
687690
for _, shard := range shards {
@@ -810,7 +813,7 @@ func (c *Ring) Watch(ctx context.Context, fn func(*Tx) error, keys ...string) er
810813

811814
for _, key := range keys {
812815
if key != "" {
813-
shard, err := c.sharding.GetByKey(hashtag.Key(key))
816+
shard, err := c.sharding.GetByKey(key)
814817
if err != nil {
815818
return err
816819
}

0 commit comments

Comments
 (0)