Fix dark cluster URI rewrite deopt storm consuming 80% CPU #1160
Open
Fix dark cluster URI rewrite deopt storm consuming 80% CPU #1160
Conversation
…append(String) is an intrinsic, a single System.arraycopy - No bimodal branching — no speculative optimization to get wrong - Monomorphic call sites — StringBuilder.append and URI.create are stable JIT targets with no polymorphic dispatch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
skipReEncodingfast path toD2URIRewriterthat constructs rewritten URIs directly from raw (already percent-encoded) components viaStringBuilder+URI.create, bypassing the expensive character-by-characterUriComponent._encodeloop.DarkClusterManagerImplonly (new D2URIRewriter(configuredURI, true)). All other callers use the default constructor and are completely unaffected.UriBuilder-based path is preserved unchanged behindskipReEncoding=false(the default).Problem
Production profiling shows
DarkClusterFilter.onRestRequest→DarkClusterManagerImpl.rewriteRequest→D2URIRewriter.rewriteURI→UriBuilder.replaceQuery→UriComponent._encodeconsuming ~80% of total CPU.The root cause is a JIT deoptimization storm on
_encode:StringBuilder sb = nullfast path vs slow path) that C2 speculatively compiles for one modeThe re-encoding is also unnecessary:
getRawQuery()returns already percent-encoded strings, so_encodescans every character only to leave them unchanged.Fix
rewriteURIFromRaweliminates the problem entirely:StringBuilder.append(String)is a JIT intrinsic (System.arraycopy)Test plan
skipReEncodingtests toTestD2URIRewriter(simple rewrite, encoded query params, no query, with fragment)skipReEncodingtests toTestDarkClusterUrlRewrite(basic, query params, encoded params, parity with default)UriBuilderpath./gradlew :d2:test --tests TestD2URIRewriter :darkcluster:test --tests TestDarkClusterUrlRewritepasses