Skip to content

perf: replace copy+reverse with index-based traversal in executeBefor…#1954

Open
tobias-ibounig-dt wants to merge 1 commit into
open-feature:mainfrom
tobias-ibounig-dt:perf/pr-3-reverse-iteration
Open

perf: replace copy+reverse with index-based traversal in executeBefor…#1954
tobias-ibounig-dt wants to merge 1 commit into
open-feature:mainfrom
tobias-ibounig-dt:perf/pr-3-reverse-iteration

Conversation

@tobias-ibounig-dt
Copy link
Copy Markdown

This PR

  • Replaces the copy-and-reverse pattern in executeBeforeHooks with an index-based reverse loop

Related Issues

None

Notes

Previously executeBeforeHooks copied the hook list into a new ArrayList and called Collections.reverse() before iterating. This allocated one extra list per flag evaluation. The change iterates the existing list in reverse using an index loop, which requires no additional allocation.

Metric main (baseline) This PR Delta
run:+totalAllocatedBytes 124,265,984 120,069,632 −4,196,352 (−3.4%)
run:+totalAllocatedInstances 2,723,316 2,654,727 −68,589 (−2.5%)

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:31
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 executeBeforeHooks method in HookSupport.java by replacing the list copying and reversal with a reverse index-based for loop. Feedback recommends adding defensive null checks for data and data.getHooks() to prevent potential NullPointerExceptions.

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 +53 to +54
List<Pair<Hook, HookContext>> hooks = data.getHooks();
for (int i = hooks.size() - 1; i >= 0; i--) {
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

To prevent potential NullPointerExceptions, it is recommended to add a defensive null check for data and data.getHooks(). Since HookSupportData is initialized with a default constructor where hooks is not set, data.getHooks() can return null if no hooks are registered or if setHooks was not called.

Suggested change
List<Pair<Hook, HookContext>> hooks = data.getHooks();
for (int i = hooks.size() - 1; i >= 0; i--) {
if (data == null || data.getHooks() == null) {
return;
}
List<Pair<Hook, HookContext>> hooks = data.getHooks();
for (int i = hooks.size() - 1; i >= 0; i--) {

…eHooks

Signed-off-by: Tobias Ibounig <tobias.ibounig@dynatrace.com>
@tobias-ibounig-dt tobias-ibounig-dt force-pushed the perf/pr-3-reverse-iteration branch from 6de83c9 to d55981c Compare June 5, 2026 08:36
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jun 5, 2026

Quality Gate Failed Quality Gate failed

Failed conditions
C 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