Skip to content

perf: skip unmodifiableMap wrapper when hookHints is empty#1953

Open
tobias-ibounig-dt wants to merge 1 commit into
open-feature:mainfrom
tobias-ibounig-dt:perf/pr-2-hook-hints
Open

perf: skip unmodifiableMap wrapper when hookHints is empty#1953
tobias-ibounig-dt wants to merge 1 commit into
open-feature:mainfrom
tobias-ibounig-dt:perf/pr-2-hook-hints

Conversation

@tobias-ibounig-dt
Copy link
Copy Markdown

This PR

  • Skips the Collections.unmodifiableMap() wrapper when hookHints is empty (the common case)

Related Issues

None

Notes

hookHints is resolved once per flag evaluation. Previously it was always wrapped in an unmodifiable view, even when empty. This PR avoids that allocation by returning Collections.emptyMap() directly when there are
no hints.

Allocation impact is negligible, but reduces on totalAllocatedInstances can be seen.

Metric main (baseline) PR 2 Delta
run:+totalAllocatedBytes 124,265,984 124,268,232 ~0 (noise)
run:+totalAllocatedInstances 2,723,316 2,691,291 −32,025 (−1.2%)

Follow-up Tasks

  • More PRs with memory improvements
  • Update benchmark.txt after all are applied

@tobias-ibounig-dt tobias-ibounig-dt requested review from a team as code owners June 5, 2026 08:27
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request optimizes the initialization of hookSupportData.hints in OpenFeatureClient.java by using Collections.emptyMap() when the hook hints are empty. A review comment points out that getHookHints() could return null, which would cause a NullPointerException when calling isEmpty(), and suggests adding a null check to handle this case safely.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +173 to +174
var hookHints = flagOptions.getHookHints();
hookSupportData.hints = hookHints.isEmpty() ? Collections.emptyMap() : Collections.unmodifiableMap(hookHints);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If flagOptions.getHookHints() returns null (which can happen if hookHints is explicitly set to null via the builder in FlagEvaluationOptions), calling hookHints.isEmpty() will throw a NullPointerException.

To ensure robust defensive programming, we should handle the null case gracefully by defaulting to Collections.emptyMap().

Suggested change
var hookHints = flagOptions.getHookHints();
hookSupportData.hints = hookHints.isEmpty() ? Collections.emptyMap() : Collections.unmodifiableMap(hookHints);
var hookHints = flagOptions.getHookHints();
hookSupportData.hints = hookHints == null || hookHints.isEmpty() ? Collections.emptyMap() : Collections.unmodifiableMap(hookHints);

Signed-off-by: Tobias Ibounig <tobias.ibounig@dynatrace.com>
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jun 5, 2026

Quality Gate Failed Quality Gate failed

Failed conditions
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

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.

1 participant