Skip to content

Performance: Avoid allocation if log level > finer#563

Open
sghill wants to merge 1 commit into
jenkinsci:masterfrom
sghill:perf/guard-finer-logs
Open

Performance: Avoid allocation if log level > finer#563
sghill wants to merge 1 commit into
jenkinsci:masterfrom
sghill:perf/guard-finer-logs

Conversation

@sghill
Copy link
Copy Markdown

@sghill sghill commented May 29, 2026

This PR stops allocating Supplier objects in a hot path if Level.FINER is not loggable.

I've been having some performance issues with Jenkins controllers as the queue size and incoming agent connections grow, similar to what's described in #560.

I profiled with JFR's settings=profile configuration, and found 20% of samples over a few hours were in LOGGER.finer(Supplier) in this code path. I know sampling isn't perfect and there are more impactful changes described in 560, but I thought this may be a nice first contribution to this plugin - it is a hot path that is allocating a Supplier on every iteration of the loop. For anyone who has the root logger set to INFO, it'd be better to avoid that allocation since it won't be logged anyway.

Screenshot 2026-05-28 at 20 30 53

Testing done

mvn clean verify and JMH Benchmarks that include the org.apache.logging.log4j:log4j-jul bridge I'm using in production. I ran my benchmarks on a Macbook Pro M1 Max with 2 forks, 5 warmup iterations and 10 benchmark iterations.

It showed an improvement of 13% with a queue size of 2000:

Screenshot 2026-05-28 at 21 23 07

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests that demonstrate the feature works or the issue is fixed

@sghill sghill requested a review from a team as a code owner May 29, 2026 04:29
Copy link
Copy Markdown
Member

@jtnord jtnord left a comment

Choose a reason for hiding this comment

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

The continued use of a supplier is dubious.
Given the expensive thing is string concatination and that is what the supplier guards against, using a supplier when we know we will log (not throw away) the message just adds an unnecessary code path.

Comment thread src/main/java/org/jenkinsci/plugins/durabletask/executors/ContinuedTask.java Outdated
Comment thread src/main/java/org/jenkinsci/plugins/durabletask/executors/ContinuedTask.java Outdated
Comment thread src/main/java/org/jenkinsci/plugins/durabletask/executors/ContinuedTask.java Outdated
Comment thread src/main/java/org/jenkinsci/plugins/durabletask/executors/ContinuedTask.java Outdated
Comment thread src/main/java/org/jenkinsci/plugins/durabletask/executors/ContinuedTask.java Outdated
@sghill sghill force-pushed the perf/guard-finer-logs branch from 5da10a1 to a6b9d4d Compare May 30, 2026 01:47
@jtnord
Copy link
Copy Markdown
Member

jtnord commented Jun 1, 2026

amends #281 which itself attempted to reduce overhead by moving from parameterized logging to a Supplier.
//cc @jglick

@jtnord jtnord requested a review from jglick June 1, 2026 09:40
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.

2 participants