Skip to content

perf: avoid unnecessary copy operation#3376

Merged
ndyakov merged 5 commits into
redis:masterfrom
cxljs:optimize/reduce-copy
May 19, 2025
Merged

perf: avoid unnecessary copy operation#3376
ndyakov merged 5 commits into
redis:masterfrom
cxljs:optimize/reduce-copy

Conversation

@cxljs

@cxljs cxljs commented May 11, 2025

Copy link
Copy Markdown
Contributor
  • reduce unnecessary copy operations in file ring.go
  • remove redundant hash operation in file ring.go

In struct Ring, the size of shards can be up to 100 (#2190 (comment)), in this case, a copy operation is called for each heartbeat, and approximately 3.1 KB data are copied. This is a waste of CPU resources. So avoiding copy operations yields benefits.

The pr #2931 reported a race here, but ​​there actually isn't one. Because every time we get the slice c.shards.list, we don't change slice itself, we only use c.shards.list[i].Client which is concurrency-safe.

@cxljs cxljs changed the title optime: reduce unnecessary copy operations optimize: reduce unnecessary copy operations May 11, 2025
@cxljs cxljs changed the title optimize: reduce unnecessary copy operations perf: avoid unnecessary copy operation May 14, 2025
Comment thread ring.go
Comment thread ring.go
@cxljs cxljs force-pushed the optimize/reduce-copy branch from 1ad81f8 to 8cd300f Compare May 15, 2025 13:11
@ndyakov ndyakov self-requested a review May 19, 2025 16:23
@ndyakov ndyakov merged commit bc70b52 into redis:master May 19, 2025
16 checks passed
ofekshenawa pushed a commit to ofekshenawa/go-redis that referenced this pull request Jun 30, 2025
* optime: reduce unnecessary copy operations

* add a comment

* trigger CI without code changes, because the bug of docker

* add comments
ofekshenawa pushed a commit that referenced this pull request Aug 10, 2025
* optime: reduce unnecessary copy operations

* add a comment

* trigger CI without code changes, because the bug of docker

* add comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants