Repository navigation
Conversation
|
Thanks @pjfanning - I tried an alternative that avoids hardcoding anything: derive a 128-bit mask
So the mask is worse in both cases - its two extra live values add register pressure. Reading the On your correctness point, I agree the constants must not silently drift from the table. Rather than Would that address the concern? |
|
@franz1981 have you seen #1682? There are probably a few different ways to do it. |
cowtowncoder
added
the
performance
|
This needs to be changed to against 3.x, if we want to proceed. |
|
@cowtowncoder #1682 covers this, at least partially. |
|
@pjfanning Can/should this be closed then? |
…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.
|
@cowtowncoder I rebased this to 3.x - it simpler than my 1682 changes so I'm happy to get it merged. |
|
@franz1981 has blocked maintainer changes to his branch. |
…ring loops Same as FasterXML#1681 for WriterBasedJsonGenerator: with the standard escape table the fast scan in _writeString2, _writeSegment and _writeString(char[]) tests constants instead of loading escCodes[c]. Custom quote char, escaped slashes and CharacterEscapes keep the lookup. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@pjfanning I see that @cowtowncoder rebased it over 3.1 once so it should work fine. |
|
Rebased on 3.x. I'm still investigating some issues that I see with Java 17 with perf regressions in some cases. Java 25: this change is positive or neutral in all scenarios. |
if you have perfasm output I can help, just need the JDK 25 and JDK 17 perfasm output or asm of the tight loops. |
|
Ran JMH perfasm/perfnorm on this change rebased onto Generated code (C2): both versions are 2x unrolled, byte-at-a-time, not vectorized.
perfnorm (per op):
The loop already runs at ~4.3 IPC, near this core's 4-wide issue limit. The 512-byte escape table always hits L1, so its load was effectively free; removing it doesn't help, while the extra compare uops cost a bit. That matches the JDK 17 A real gain probably needs fewer instructions per char (e.g. SWAR over 8 chars, or a vectorized Latin-1 String path) rather than a different per-char check. |
|
@franz1981 thanks! Here is the scenario. JMH benchmark (standalone project; jackson-core jar swapped on the classpath: StringWriteBench.javapackage bench;
import java.io.OutputStream;
import java.io.Writer;
import java.util.Random;
import java.util.concurrent.TimeUnit;
import org.openjdk.jmh.annotations.*;
import org.openjdk.jmh.infra.Blackhole;
import tools.jackson.core.JsonGenerator;
import tools.jackson.core.ObjectWriteContext;
import tools.jackson.core.json.JsonFactory;
@State(Scope.Thread)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.MICROSECONDS)
@Warmup(iterations = 5, time = 1)
@Measurement(iterations = 5, time = 1)
@Fork(2)
public class StringWriteBench
{
static final int COUNT = 200;
/** ascii = no escapes; escaped = one '"' near the end of each value */
@Param({"ascii", "escaped"})
public String content;
@Param({"12", "100", "1000"})
public int length;
final JsonFactory f = new JsonFactory();
String[] values;
char[][] charValues;
static final OutputStream NULL_OUT = new OutputStream() {
@Override public void write(int b) { }
@Override public void write(byte[] b, int off, int len) { }
};
static final Writer NULL_W = new Writer() {
@Override public void write(char[] c, int off, int len) { }
@Override public void write(String s, int off, int len) { }
@Override public void flush() { }
@Override public void close() { }
};
@Setup
public void setup() {
Random r = new Random(123);
String alphabet = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789 .,:;-_/";
values = new String[COUNT];
charValues = new char[COUNT][];
for (int i = 0; i < COUNT; ++i) {
char[] c = new char[length];
for (int j = 0; j < length; ++j) {
c[j] = alphabet.charAt(r.nextInt(alphabet.length()));
}
if ("escaped".equals(content)) {
c[length - 2] = '"';
}
values[i] = new String(c);
charValues[i] = c;
}
}
@Benchmark
public void utf8String(Blackhole bh) {
try (JsonGenerator g = f.createGenerator(ObjectWriteContext.empty(), NULL_OUT)) {
g.writeStartArray();
for (String s : values) g.writeString(s);
g.writeEndArray();
}
}
@Benchmark
public void utf8Chars(Blackhole bh) {
try (JsonGenerator g = f.createGenerator(ObjectWriteContext.empty(), NULL_OUT)) {
g.writeStartArray();
for (char[] c : charValues) g.writeString(c, 0, c.length);
g.writeEndArray();
}
}
@Benchmark
public void writerString(Blackhole bh) {
try (JsonGenerator g = f.createGenerator(ObjectWriteContext.empty(), NULL_W)) {
g.writeStartArray();
for (String s : values) g.writeString(s);
g.writeEndArray();
}
}
@Benchmark
public void writerChars(Blackhole bh) {
try (JsonGenerator g = f.createGenerator(ObjectWriteContext.empty(), NULL_W)) {
g.writeStartArray();
for (char[] c : charValues) g.writeString(c, 0, c.length);
g.writeEndArray();
}
}
}Run, e.g.: ( JDK 17 (17.0.20 Temurin), x86 i7-8550U — hot loop of 3.xThis PRBoth are 2x unrolled, byte-at-a-time, no vectorization. With the PR, the |
|
@pjfanning and what JDK 25 does differently? you have the perfasm and perfnorm there? |
|
JDK 25 (25.0.4 Temurin), same machine/benchmark ( 3.x: 2x unrolled, same shape as JDK 17This PR: 4x unrolledDifferences from JDK 17:
The JDK 17 loops in my previous comment are both 2x unrolled with no spills, so the regression there looks like the plain extra compare work rather than a spill problem. |
|
yep @pjfanning the 4X is what I have observed, but didn't observed register spilling if the caller is small: did you have a |
|
@franz1981 answers + more JDK 17 data (JDK 17 is where this PR regresses vs Inlining: no, my benchmark had no JDK 17 perfnorm (dontinline, ascii, length 1000, per op, i7-8550U):
So it is front-end, as you suspected, but not from more taken branches: the work, branch counts and uop source (DSB) are all the same. The extra ~191K undelivered slots (about 48K cycles) account for most of the +70K cycles. JCC erratum check (
This CPU has the JCC microcode fix. Without C2's padding, the loop falls out of the DSB onto the legacy decoders (MITE), and both versions get 50-70% slower. With padding (the default), my guess is that the PR's 3 macro-fused cmp/jcc pairs per char, plus padding, sit worse in 32-byte DSB windows, so delivery per cycle drops. That would also explain why results differ between machines. I couldn't test 32-byte loop alignment: JDK 25 avoids this by unrolling 4x, which halves taken branches (52K vs 101K per op). Next I'll try two variants on JDK 17: (1) fewer branches per char, and (2) a manual 4x unroll. |
|
Tried two variants aimed at the JDK 17 front-end issue, in a standalone microbenchmark (one
ns per string (lower is better):
Next I'll re-check a different approach: scan first, then bulk-copy with |
|
@pjfanning reproduced this on different hardware — Ryzen 9 7950X pinned at 4.3 GHz ( All numbers below use Repeatability was tight — the jdk17/len=100 PR series was 17.986 / 17.985 / 17.975, and five of the six series held to ±0.1 us/op across reps. So on this machine JDK 17 regresses only at length 100, and is faster at length 1000 — the opposite sign to your +5.7% at length 1000. Might be worth checking more than one length before tuning against the regression. Codegen under
That makes your planned variant (2) — the manual 4x unroll on JDK 17 — look like the right lever: 4x vs 2x is exactly what separates the two JDKs here. Also worth noting: this is Zen 4, which has no JCC erratum and no Finally, three traps I fell into while reading this assembly — I reported wrong numbers from all three before catching them, so in case they save anyone else time:
|
|
Clean re-run (3 forks, idle machine) of a different approach for the
I'll try this in |
|
@pjfanning on JDK 27 IIRC there should be a new String method to know the decodedLength but IDK if can help here |
|
Tried your scan + I applied it to // Scan only: find the first char needing escaping, no per-char store.
int i = offset;
while (i < len) {
int ch = text.charAt(i);
if (ch > 0x7F || escCodes[ch] != 0) {
break;
}
++i;
}
if (i > offset) {
text.getBytes(offset, i, outputBuffer, outputPtr);
outputPtr += (i - offset);
offset = i;
}Checked first that it is byte-identical to
Two differences from your table worth noting:
Assembly (hsdis decoded, loop in Removing the store does two things at once: it roughly halves the per-character instruction count, and on JDK 17 it is what finally lets C2 unroll 4x — neither One small thing: the scan loop still carries the |
|
#1775 is what Claude AI found to work best on one of my machines, an old HP laptop with WSL2 so I do most dev on Ubuntu. |
|
Promised numbers for the fourth variant — constant-check scan +
( Your observation holds on JDK 17 and does not on JDK 25, and the assembly says why. Main 4x-unrolled scan loop in On JDK 17 the constant check costs 2.5 more instructions per character (13.00 vs 10.50), which matches the measured +22% at length 1000. On JDK 25 the two loops are the same length — 37 instructions for 4 characters either way — and the measurements are tied (81.86 vs 81.64). So on 25, trading the table load for three immediate compares is free in the scan loop. That matters beyond x86: on aarch64 a table lookup needs the table's base address materialised into a register ( One thing I tried and should report as a negative: both variants above still have the method's original prologue, which hoists |
|
@franz1981 so are you proposing to change this PR or are you sticking with the existing code? |
Segments under 32 chars use the constant comparison (standard escape table only) from FasterXML#1681; longer ones keep the scan + bulk-copy path. Co-authored-by: Francesco Nigro <nigro.fra@gmail.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
📈 Overall Code Coverage
|
This branch has not been deployed
3 participants
_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.