Skip to content

Perf: Remove redundant map lookups in CustomTierSelectorStrategy comparator#19052

Merged
abhishekrb19 merged 1 commit intoapache:masterfrom
abhishekrb19:perf_custom_tier
Feb 26, 2026
Merged

Perf: Remove redundant map lookups in CustomTierSelectorStrategy comparator#19052
abhishekrb19 merged 1 commit intoapache:masterfrom
abhishekrb19:perf_custom_tier

Conversation

@abhishekrb19
Copy link
Copy Markdown
Contributor

Avoid repeated containsKey() and get() calls by retrieving each map value once and using null checks instead. This reduces redundant lookups and simplifies the comparator logic.

This PR has:

  • been self-reviewed.
  • been tested in a test Druid cluster.

Just use get() rather than containsKey() multiple times along with .get().
Copy link
Copy Markdown
Contributor

@GWphua GWphua left a comment

Choose a reason for hiding this comment

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

LGTM, regardless of the answer to the question below.

How much of a performance boost is this, given that it's looking up a hashmap?

@abhishekrb19
Copy link
Copy Markdown
Contributor Author

Thanks for taking a look, @GWphua!

How much of a performance boost is this, given that it's looking up a hashmap?

I didn’t benchmark this change. It should remove about 2–3 unnecessary map lookups per query.

@abhishekrb19 abhishekrb19 merged commit e12a63a into apache:master Feb 26, 2026
81 of 83 checks passed
@abhishekrb19 abhishekrb19 deleted the perf_custom_tier branch February 26, 2026 16:48
@abhishekrb19
Copy link
Copy Markdown
Contributor Author

I got curious about the performance improvement and ran a quick benchmark. With this lookup access pattern, we see a good ~18–20% speed up on average - CustomTierSelectorComparatorBenchmark.java. I did a quick spot check in other areas of the codebase and there seems to be a few other occurrences where we can simply do map.get(key) instead of map.containsKey(key) + map.get(key).

Benchmark                                                Mode  Cnt    Score   Error  Units
CustomTierSelectorComparatorBenchmark.newImplementation  avgt   10  456.969 ± 5.175  ns/op
CustomTierSelectorComparatorBenchmark.oldImplementation  avgt   10  571.750 ± 4.194  ns/op

@cecemei cecemei added this to the 37.0.0 milestone Apr 8, 2026
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.

3 participants