Skip to content

[PERF] Remote-write: re-use memory to read WAL data#16197

Merged
bwplotka merged 1 commit intoprometheus:mainfrom
bboreham:watcher-no-shadow
Mar 11, 2025
Merged

[PERF] Remote-write: re-use memory to read WAL data#16197
bwplotka merged 1 commit intoprometheus:mainfrom
bboreham:watcher-no-shadow

Conversation

@bboreham
Copy link
Member

The := causes new variables to be created, which means the outer slice stays at nil, and new memory is allocated every time round the loop.

Extracted from #16182
Credit to @bwplotka.

The `:=` causes new variables to be created, which means the outer
slice stays at nil, and new memory is allocated every time round the
loop.

Extracted from prometheus#16182
Credit to @bwplotka.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
@bboreham
Copy link
Member Author

@mmorel-35 is there a linter that will catch variable shadowing like this?

@mmorel-35
Copy link
Contributor

mmorel-35 commented Mar 10, 2025

shadow rule from govet could have spotted this indeed
According to this try there are 296 issues of that kind on Prometheus.

@miledxz
Copy link
Contributor

miledxz commented Mar 10, 2025

@mmorel-35 thanks for sharing the insights, I can try to fix these in smaller commits

@miledxz
Copy link
Contributor

miledxz commented Mar 11, 2025

a little bit more context here: #16198

@bboreham
Copy link
Member Author

To continue the thread here, #16198 is not obviously an improvement.

Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

In my PR I also added a clear interface comments on why this should be safe (no one reusing those slices as implementer). That's fine to merge those comments later (as long as eventually). I also double checked the implementers, no one is reusing those slices, so.. let's merge, thanks! 💪🏽

@bwplotka bwplotka merged commit 30d0479 into prometheus:main Mar 11, 2025
27 checks passed
machine424 pushed a commit to machine424/prometheus that referenced this pull request Mar 26, 2025
The `:=` causes new variables to be created, which means the outer
slice stays at nil, and new memory is allocated every time round the
loop.

Extracted from prometheus#16182
Credit to @bwplotka.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
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.

4 participants