Skip to content

Commit 86b6e6f

Browse files
bluestreak01claude
andcommitted
perf(ilp): replace O(N²) SF segment sort at open
SegmentLog.scanDirectory used insertion sort over the segments list. At the documented sf_max_total_bytes / sf_max_bytes ceiling (1 TiB / 64 MiB ≈ 16K segments) that is ~268M comparisons + array shifts → multi- second wall time before the I/O thread can start. Replaced with an in-place quicksort with median-of-three pivot. O(N log N) average, no allocation (matching the surrounding code's discipline), recursion depth bounded by ~2 log₂(N) by always recursing into the smaller partition and looping on the larger. Median-of-three is required because the insertion sort's only saving grace was O(N) on already-sorted input, which is the common case from readdir on filesystems that return entries in lexicographic (and therefore baseSeq-hex) order. A naive first-element-pivot quicksort would degrade back to O(N²) in exactly that scenario. Tests: - testLargeSegmentCountReopensInOrder (SegmentLogTortureTest): generates 1024 sealed segments, reopens, asserts the replay returns every appended seq exactly once in order, and that reopen+replay completes within a generous wall-clock bound that catches a regression back to O(N²) at the production ceiling. - Full SF + adjacent suite: 117/117 pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent cf3152d commit 86b6e6f

2 files changed

Lines changed: 125 additions & 12 deletions

File tree

core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/SegmentLog.java

Lines changed: 60 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -404,18 +404,13 @@ private void scanDirectory() {
404404
ff.findClose(find);
405405
}
406406

407-
// Insertion sort by baseSeq. Open-time only and N is typically small
408-
// (one active segment plus a handful of unacked sealed segments), so
409-
// the O(N^2) is irrelevant and avoids the java.util.Comparator alloc.
410-
for (int i = 1, n = segments.size(); i < n; i++) {
411-
Segment x = segments.getQuick(i);
412-
int j = i - 1;
413-
while (j >= 0 && Long.compareUnsigned(segments.getQuick(j).baseSeq, x.baseSeq) > 0) {
414-
segments.setQuick(j + 1, segments.getQuick(j));
415-
j--;
416-
}
417-
segments.setQuick(j + 1, x);
418-
}
407+
// Open-time sort by baseSeq. Worst case is `sf_max_total_bytes /
408+
// sf_max_bytes` segments — at the documented limit (1 TiB / 64 MiB)
409+
// that is ~16K entries, where the previous insertion sort spent
410+
// multiple seconds in O(N²) compares + array shifts. In-place
411+
// quicksort with median-of-three pivot keeps the no-allocation
412+
// discipline of the surrounding code.
413+
sortSegmentsByBaseSeq(0, segments.size());
419414

420415
// Validate: at most one active segment, and only as the last entry.
421416
for (int i = 0, n = segments.size(); i < n; i++) {
@@ -698,6 +693,59 @@ private void ensureOpen() {
698693
}
699694
}
700695

696+
/**
697+
* In-place quicksort over {@code segments[lo, hi)} keyed by unsigned
698+
* {@code baseSeq}. Median-of-three pivot selection avoids the
699+
* pathological O(N²) on already-sorted input that {@code readdir} on
700+
* many filesystems produces. Recursion depth is bounded by ~2 log₂(N);
701+
* for the documented 16K-segment ceiling that is well under the JVM
702+
* default stack.
703+
*/
704+
private void sortSegmentsByBaseSeq(int lo, int hi) {
705+
while (hi - lo > 1) {
706+
int mid = (lo + hi) >>> 1;
707+
long a = segments.getQuick(lo).baseSeq;
708+
long b = segments.getQuick(mid).baseSeq;
709+
long c = segments.getQuick(hi - 1).baseSeq;
710+
// Median of {a, b, c} → pivot index.
711+
int pivotIdx;
712+
if (Long.compareUnsigned(a, b) < 0) {
713+
if (Long.compareUnsigned(b, c) < 0) pivotIdx = mid;
714+
else if (Long.compareUnsigned(a, c) < 0) pivotIdx = hi - 1;
715+
else pivotIdx = lo;
716+
} else {
717+
if (Long.compareUnsigned(a, c) < 0) pivotIdx = lo;
718+
else if (Long.compareUnsigned(b, c) < 0) pivotIdx = hi - 1;
719+
else pivotIdx = mid;
720+
}
721+
long pivot = segments.getQuick(pivotIdx).baseSeq;
722+
swapSegments(pivotIdx, hi - 1);
723+
int store = lo;
724+
for (int i = lo; i < hi - 1; i++) {
725+
if (Long.compareUnsigned(segments.getQuick(i).baseSeq, pivot) < 0) {
726+
swapSegments(i, store++);
727+
}
728+
}
729+
swapSegments(store, hi - 1);
730+
// Recurse on the smaller partition; loop on the larger to keep
731+
// recursion depth bounded by log₂(N).
732+
if (store - lo < hi - store - 1) {
733+
sortSegmentsByBaseSeq(lo, store);
734+
lo = store + 1;
735+
} else {
736+
sortSegmentsByBaseSeq(store + 1, hi);
737+
hi = store;
738+
}
739+
}
740+
}
741+
742+
private void swapSegments(int i, int j) {
743+
if (i == j) return;
744+
Segment tmp = segments.getQuick(i);
745+
segments.setQuick(i, segments.getQuick(j));
746+
segments.setQuick(j, tmp);
747+
}
748+
701749
/** Parse `<baseSeq>.sfa` or `<baseSeq>-<lastSeq>.sfs`. Returns null for unrecognized names. */
702750
private Segment parseFilename(String name) {
703751
try {

core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/SegmentLogTortureTest.java

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,71 @@ public void testTruncationAcrossMultipleSegments() throws Exception {
479479
});
480480
}
481481

482+
/**
483+
* Open-time sort regression: at the documented {@code sf_max_total_bytes
484+
* / sf_max_bytes} ceiling (~16K segments) the previous insertion sort
485+
* over {@code segments} ran in O(N²) and burnt multi-second wall time
486+
* before the I/O thread could even start. The test creates 1024 sealed
487+
* segments by forcing one-frame-per-segment via a tiny per-segment cap,
488+
* reopens, and asserts:
489+
* <ul>
490+
* <li>every appended sequence is replayed exactly once, in order;</li>
491+
* <li>{@code nextSeq()} matches the total appended frame count;</li>
492+
* <li>reopen + replay completes within a generous wall-clock bound
493+
* that the old O(N²) sort would still satisfy at this scale, but
494+
* that catches a regression pushing back into multi-second land
495+
* for the documented production ceiling (~16K segments).</li>
496+
* </ul>
497+
*/
498+
@Test
499+
public void testLargeSegmentCountReopensInOrder() throws Exception {
500+
TestUtils.assertMemoryLeak(() -> {
501+
// maxBytes = HEADER_SIZE + FRAME_HEADER_SIZE + payload = 24+8+16 = 48.
502+
// First frame fits in segment 0; every subsequent frame triggers
503+
// rotation. 1024 frames → ~1023 sealed + 1 active = 1024 segments.
504+
final int frameCount = 1024;
505+
final int payloadSize = 16;
506+
final long maxBytes = 48;
507+
508+
long buf = Unsafe.malloc(payloadSize, MemoryTag.NATIVE_DEFAULT);
509+
try {
510+
for (int i = 0; i < payloadSize; i++) {
511+
Unsafe.getUnsafe().putByte(buf + i, (byte) (i & 0xff));
512+
}
513+
try (SegmentLog log = SegmentLog.open(tmpDir, maxBytes)) {
514+
long lastSeq = -1;
515+
for (int i = 0; i < frameCount; i++) {
516+
lastSeq = log.append(buf, payloadSize);
517+
}
518+
assertEquals(frameCount - 1, lastSeq);
519+
log.fsync();
520+
}
521+
} finally {
522+
Unsafe.free(buf, payloadSize, MemoryTag.NATIVE_DEFAULT);
523+
}
524+
525+
long startMs = System.currentTimeMillis();
526+
try (SegmentLog log2 = SegmentLog.open(tmpDir, maxBytes)) {
527+
assertEquals(frameCount, log2.nextSeq());
528+
final long[] expected = {0L};
529+
final int[] count = {0};
530+
log2.replay((seq, addr, len) -> {
531+
assertEquals("frame seq out of order at index " + count[0],
532+
expected[0], seq);
533+
expected[0]++;
534+
count[0]++;
535+
return true;
536+
});
537+
assertEquals("replayed " + count[0] + " frames, expected " + frameCount,
538+
frameCount, count[0]);
539+
}
540+
long elapsedMs = System.currentTimeMillis() - startMs;
541+
assertTrue("reopen+replay took " + elapsedMs + "ms (expected < 5000ms); " +
542+
"regression suggests scanDirectory's segment sort is back to O(N²)",
543+
elapsedMs < 5_000);
544+
});
545+
}
546+
482547
private static void appendBytes(SegmentLog log, byte[] bytes) {
483548
long buf = Unsafe.malloc(bytes.length, MemoryTag.NATIVE_DEFAULT);
484549
try {

0 commit comments

Comments
 (0)