Skip to content

Use field updaters for timeout guards#2263

Closed
pavel-ptashyts wants to merge 1 commit into
AsyncHttpClient:mainfrom
maygemdev:perf/timeout-field-updaters
Closed

Use field updaters for timeout guards#2263
pavel-ptashyts wants to merge 1 commit into
AsyncHttpClient:mainfrom
maygemdev:perf/timeout-field-updaters

Conversation

@pavel-ptashyts

@pavel-ptashyts pavel-ptashyts commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Summary

  • replace per-instance timeout AtomicBoolean guards with volatile int fields managed by static AtomicIntegerFieldUpdaters
  • preserve existing run-once, reschedule, and cleanup semantics
  • add unit coverage for idempotent timeout task cleanup

Verification

  • ./mvnw -pl client -DskipTests compile
  • ./mvnw -pl client -Dtest=TimeoutTimerTaskTest,PerRequestTimeoutTest,NettyConnectListenerTest test

Attribution

Codex on behalf of Pavel Ptashyts

Replace per-instance AtomicBoolean guards in TimeoutsHolder and
TimeoutTimerTask with volatile int fields managed by static
AtomicIntegerFieldUpdaters. This preserves run-once and cleanup
semantics while avoiding an extra guard object allocation per timeout
holder and task.

Add unit coverage for idempotent TimeoutTimerTask cleanup.

Codex on behalf of Pavel Ptashyts

Co-Authored-By: Codex <codex@openai.com>
@pavel-ptashyts
pavel-ptashyts force-pushed the perf/timeout-field-updaters branch from 19a8b49 to 863afec Compare July 20, 2026 08:50
@pavel-ptashyts

Copy link
Copy Markdown
Contributor Author

Closing this for the 3.x line because Revapi correctly reports the removal of the protected TimeoutTimerTask.done field as an incompatible API change. Retaining the AtomicBoolean field would retain the allocation this change is intended to remove, while adding a Revapi exception would silently break external subclasses. This optimization can be reconsidered for a major release; the branch is retained as a reference.

@pavel-ptashyts
pavel-ptashyts deleted the perf/timeout-field-updaters branch July 20, 2026 14:35
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