Skip to content

internal: Improve the Iter performance of LockMap on empty maps#2130

Merged
knusbaum merged 2 commits into
mainfrom
andrew.glaude/CherryPickMorePerf
Jul 20, 2023
Merged

internal: Improve the Iter performance of LockMap on empty maps#2130
knusbaum merged 2 commits into
mainfrom
andrew.glaude/CherryPickMorePerf

Conversation

@ajgajg1134

Copy link
Copy Markdown
Contributor

Turns out doing RLock and RUnlock can still be slow especially when this code is in a HOT path. For most users this map will be empty. Therefore it's worth speeding up that case at the cost of a tiny bit of extra memory and a bit of complexity.

What does this PR do?

Cherry-picks across #2127

Reviewer's Checklist

  • Changed code has unit tests for its functionality.
  • If this interacts with the agent in a new way, a system test has been added.

Turns out doing RLock and RUnlock can still be slow especially when this code is in a HOT path. For most users this map will be empty. Therefore it's worth speeding up that case at the cost of a tiny bit of extra memory and a bit of complexity.
@ajgajg1134
ajgajg1134 marked this pull request as ready for review July 20, 2023 14:35
@ajgajg1134
ajgajg1134 requested a review from a team as a code owner July 20, 2023 14:35
@pr-commenter

pr-commenter Bot commented Jul 20, 2023

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2023-07-20 15:56:37

Comparing candidate commit ae9d31a in PR branch andrew.glaude/CherryPickMorePerf with baseline commit 74ef48b in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 27 metrics, 0 unstable metrics.

@knusbaum
knusbaum merged commit 66d191b into main Jul 20, 2023
@knusbaum
knusbaum deleted the andrew.glaude/CherryPickMorePerf branch July 20, 2023 16:16
katiehockman pushed a commit that referenced this pull request Jul 26, 2023
Turns out doing RLock and RUnlock can still be slow especially when this code is in a HOT path. For most users this map will be empty. Therefore it's worth speeding up that case at the cost of a tiny bit of extra memory and a bit of complexity.

Cherry-picks across #2127
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants