Skip to content

Fix #1680: drop the per-character escape-table load in the ASCII copy loop - #1681

Open
franz1981 wants to merge 1 commit into
FasterXML:3.1from
franz1981:3.1
Open

franz1981 wants to merge 1 commit into
FasterXML:3.1from
franz1981:3.1

Conversation

@franz1981

Copy link
Copy Markdown

_writeStringSegment() reads _outputEscapes[ch] per character. For the standard table the same test is constant-expressible, so the loop now branches on a loop-invariant flag that C2 unswitches and constant-folds; the standard clone has no table load. Custom quote char, escaped slashes and CharacterEscapes are unaffected -- different array, flag false, original lookup.

Shorter body lets C2 unroll 4 characters per iteration instead of 2: 15.5 ->
13.8 instructions and 2 -> 1 stack operands per character on JDK 25/x86_64. Writing one String property per object: 356.5 -> 347.5 ns/op (3.1.5).

Measure after databind#6182: with that present the loop is not unrolled at all and this change appears to be worth ~23%, which is mostly that issue and not the removed load.

Output verified byte-identical over chars 0x00-0x100, quotes, backslashes, control chars and strings crossing the segment boundary.

…SCII copy loop

_writeStringSegment() reads _outputEscapes[ch] per character. For the standard
table the same test is constant-expressible, so the loop now branches on a
loop-invariant flag that C2 unswitches and constant-folds; the standard clone
has no table load. Custom quote char, escaped slashes and CharacterEscapes are
unaffected -- different array, flag false, original lookup.

Shorter body lets C2 unroll 4 characters per iteration instead of 2: 15.5 ->
13.8 instructions and 2 -> 1 stack operands per character on JDK 25/x86_64.
Writing one String property per object: 356.5 -> 347.5 ns/op (3.1.5).

Measure after databind#6182: with that present the loop is not unrolled at all
and this change appears to be worth ~23%, which is mostly that issue and not
the removed load.

Output verified byte-identical over chars 0x00-0x100, quotes, backslashes,
control chars and strings crossing the segment boundary.
@franz1981
franz1981 changed the base branch from 3.x to 3.1 September 1, 2026 14:44
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

📈 Overall Code Coverage

Metric Coverage Change
Instructions coverage 83.92% 📈 +0.010%
Branches branches 76.97% 📈 +0.060%

Overall project coverage from JaCoCo test results. Change values compare against the latest base branch build.

@franz1981

Copy link
Copy Markdown
Author

Thanks @pjfanning - I tried an alternative that avoids hardcoding anything: derive a 128-bit mask
(two longs) from whatever _outputEscapes is in effect, and test it with a shift. It would have
covered custom quote chars and CharacterEscapes too. Measured on the reproducer with real
jackson-core 3.1.5, SingleBench.serialize, JDK 25, 3 forks:

clean profile String.charAt profile polluted
stock 370.6 ± 42.7 444.9 ± 5.3
this PR 344.5 ± 11.5 345.2 ± 4.7
mask 381.6 ± 6.8 552.4 ± 20.8

So the mask is worse in both cases - its two extra live values add register pressure. Reading the
compiled loop explains why this PR helps: when the profile is polluted C2 cannot unroll, and then
the stock loop spills 13 stack operands per character and the mask 18, while this PR spills zero.
Dropping the table reference is what keeps the live set in registers. That is also why it is flat
across profiles while stock degrades 20%.

On your correctness point, I agree the constants must not silently drift from the table. Rather than
supporting or removing _outputEscapes, I can make the duplication self-verifying: compute a
static final boolean once by checking the predicate against get7BitOutputEscapes() over 0..127,
and require it in addition to the identity check, so if the default table ever changes the fast path
simply switches itself off instead of changing output. Plus a unit test asserting equivalence for
the standard table, escaped slashes, a custom quote char and CharacterEscapes.

Would that address the concern?

@pjfanning

Copy link
Copy Markdown
Member

@franz1981 have you seen #1682? There are probably a few different ways to do it.

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