feat: performance improvement (Do not merge maps if the globalForcedVariations or userForcedVariations are empty…#196
Conversation
gaurav-rana
changed the title
gaurav-rana
changed the title
gaurav-rana
changed the title
|
Hi @gaurav-rana |
|
Hi @vazarkevych can you please review the PR, its ready for review now! |
vazarkevych
left a comment
vazarkevych
left a comment
There was a problem hiding this comment.
Hi, overall looks good! One small suggestion in mergeMaps method. After fix we'll merge it and prepare release
| ); | ||
|
|
||
| public static <K, V> Map<K, V> mergeMaps(@Nullable Map<K, V> base, @Nullable Map<K, V> overrides) { | ||
| if (base == null || base.isEmpty()) return overrides != null ? new HashMap<>(overrides) : Collections.emptyMap(); |
There was a problem hiding this comment.
Let's return new HashMap<>() instead of Collections.emptyMap() for consistency — so the method always returns a mutable map and we prevent a potential UnsupportedOperationException if someone mutates the result in the future.
|
@gaurav-rana Thanks for your contributions. Appreciate it. We have pulled your changes in and added a few improvements. See PR #199. Once it goes through the review process, we'll release it and let you know. Thank you! |
3 participants
… or null)
The stream operations are usually expensive, in our backend app we are observing that 25% of overall GB related overhead is coming from
GrowthbookUtils.mergeMapsmethod. In our case, we dont use feature overrides, so the maps are always empty.