Skip to content

net: remove the quadratic copy from bulk chunk assembly - #204

Merged
EverNife merged 3 commits into
CrucibleMC:stagingfrom
EverNife:feature/chunk-send-single-buffer
Sep 13, 2026
Merged

EverNife merged 3 commits into
CrucibleMC:stagingfrom
EverNife:feature/chunk-send-single-buffer

Conversation

@EverNife

@EverNife EverNife commented Sep 7, 2026

Copy link
Copy Markdown
Member

What

Three changes to the chunk streaming path, all on the server side of a join or a teleport.

  • 2e33e131 — S26PacketMapChunkBulk measures every chunk first, allocates the raw buffer once
    at its exact size, and writes each chunk straight into it.
  • 24b5acf4 — S21PacketChunkData sizes its deflate output with zlib's compressBound instead
    of the input length, and throws if deflate() still stops short.
  • d7175306 — crucible.optimization.maxChunkSendsPerTick defaults to 20 instead of 5.

max-bulk-chunks (spigot.yml) is not touched. It stays at Spigot's default of 5.

The quadratic copy is pre-existing

The growth loop in the S26PacketMapChunkBulk constructor arrived with the Thermos import
(3d98f862) and nothing recent touched it. For a packet of k chunks it reallocates the
concatenation buffer k times and copies everything accumulated so far on each pass — roughly k²/2
chunk copies where k would do. With 51 KB chunks that is ~510 KB of memcpy for a 5-chunk packet
and ~62 MB for a 50-chunk one, all on the server thread.

What is new is how often that cost is paid, and how large k can get:

  • A tick used to send a single bulk packet. It now sends several, and this PR raises the per-tick
    budget to 20, so the block serializes 20 chunks per tick where it used to serialize 5.
  • max-bulk-chunks now sizes the packet independently of the tick budget, which makes raising it
    the natural way to cut packet count. That is exactly the direction the quadratic punishes — it
    is the k in k².

So this is not repairing a regression. It removes a cost the throughput work would otherwise
multiply, and it is what makes raising the per-tick default safe rather than merely faster.

The deflate sizing is pre-existing too, and comes from vanilla

S21PacketChunkData sized its output buffer at the input length. That is not a bound on deflate
output: at low levels zlib stores rather than compresses and the output grows past the input, and
the near-empty payload the unload path sends (PlayerManager ships one S21PacketChunkData per
unloaded chunk with no sections selected) is small enough for the zlib header and trailer alone to
exceed it. deflate() then writes what fits and returns without finishing, and the packet carries
a truncated stream that the client inflates into an empty chunk — no error on either side.

The configurable deflate level from #203 widens this, since level 0 always stores. The fix uses
zlib's own bound and turns a silent truncation into a loud exception.

Why 20 chunks per tick

Forge sends 5 per tick, and 5 was also its packet size — one bulk packet per tick. Here the two
are separate settings, so the budget no longer has to match the packet size, and 20 drains a join
queue in a quarter of the ticks.

Every chunk in the budget is serialized on the server thread before it leaves, which is why this
default is tied to the first change. At 20 chunks per tick that block costs ~0.70 ms per streaming
player with the quadratic copy in place, and ~0.31 ms without it. The old default of 5 is what kept
the old cost tolerable; removing the cost is what makes 20 reasonable.

max-bulk-chunks stays at 5. Raising it is now nearly free on the server thread, but it is a
Spigot key and changing its default is a separate decision. SpigotWorldConfig already warns when
it is set above the per-tick budget, since it then sizes a packet that cannot be filled.

Measured

A/B on one build with only the constructor swapped, same world and same session; five drains of
225 chunks per configuration, first drain dropped as JIT-cold. Medians of server-thread time in
the chunk send block, per drain:

chunks per packet before after
5 7.87 ms 3.46 ms
50 41.15 ms 4.07 ms

Going from 5 to 50 chunks per packet cost 5.23x before and costs 1.17x now. At 50 the
server-thread cost drops 10.1x.

Deflate throughput is unchanged, 3.2–3.6 ms per MB of raw chunk data on both sides — it runs on
the netty threads and neither change touches it.

Across the same runs: no NullPointerException, no truncated-deflate trip, no frame overflow, and
the client stayed connected through every drain, which is what rules out a short stream — a
truncated deflate surfaces as empty chunks or a dropped client, never as a server-side error.

Compatibility

The wire format is unchanged: same packet, same fields, same order. Only the staging of the bytes
before deflate differs. S26PacketMapChunkBulk no longer keeps the per-chunk byte[] on the
server — readPacketData still fills them for the client, and serialize() reads the recorded
lengths instead.

The per-chunk payload arrays only fed a debug string, and the concat buffer
regrew on every chunk; together they tripled the allocation per packet.
The bulk packet already did this; the single chunk packet still deflated
into a buffer the size of its input, so a low level or incompressible data
would ship a truncated stream.
Forge's 5 doubled as the packet size, so it had to stay low; the two are
separate settings now and the packet this feeds is still capped by
max-bulk-chunks.
@EverNife

EverNife commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

Validated on a production-sized modpack

The description's numbers come from a small synthetic world. Repeated here on a real server to
check the win survives chunks that actually contain machines.

Environment: a live skyblock snapshot — 104 mods, 57 plugins, 15 GB world, on Java 21. Among
them IndustrialCraft 2, Thermal Expansion / Foundation / Dynamics, Applied Energistics 2,
Thaumcraft 4, EnderIO, Mekanism, Draconic Evolution, Botania, Big Reactors, Blood Magic, Witchery,
Extra Utilities. Three clients joined at once and were driven through the hub and ten different
player islands
. Every drain is 225 chunks. Both sides are one jar with only the
S26PacketMapChunkBulk constructor swapped.

At max-bulk-chunks: 50

Medians over three dense hub drains per side:

before after
assembly per drain 42.313 ms 1.963 ms
assembly as a share of the send block 82.2 % 24.7 %
whole send block per drain 50.159 ms 9.023 ms

Assembly drops 21.6x, the whole block 5.6x. A floor, not a ceiling — the new side was
carrying 1.6-3.9x more raw data (9.77-19.54 MB against 5.04-5.99 MB). At this packet size the
queue drains in one tick: that tick used to spend ~50 ms, the entire budget, on one joining
player. It now spends ~9 ms.

At the shipped default (max-bulk-chunks: 5)

No measurable difference — 1.06x, inside the noise. Expected: with five chunks per packet the
quadratic copy has little room, assembly is ~28 % of the block, and the rest is chunk I/O neither
side touches.

So this PR does not speed up a stock server. What it does is make max-bulk-chunks safe to raise
— it removes the penalty, it does not create a reason.

Still recommend keeping it at 5

Batching more chunks per packet does not buy bandwidth. Measured compression: 11.88x at k=5
against 11.93x at k=50 on hub chunks, and slightly worse on the synthetic world. zlib's window
is 32 KB and one chunk is already ~51 KB, so the compressor never sees across a chunk boundary.
All that a higher k buys is packet count.

There is also a ceiling: the length prefix is 3 VarInt bytes, so a packet cannot exceed 2 MB
after deflate
(unable to fit N into 3 server-side, length wider than 21-bit client-side).
With real chunks at the default level 2 this is very hard to reach — k=50 produced 85-330 KB
packets, 6-24x of headroom. The way to actually hit it is
chunkCompressionLevel: 0, where zlib stores instead of compressing: worst-case chunks are
196 KB each, so the ceiling arrives around k=10. Either way it fails loudly, never silently.

Correctness

Across every run: no NullPointerException, no truncated-deflate trip, no frame overflow, clients
connected through all ten island transitions and back. Identical compression ratio on both sides
confirms byte-identical payloads.

Worth noting the silent failure this removes: the old code sized the deflate output at
input.length, which is not a bound on deflate output. When it overflowed, deflate() wrote what
fit and returned unfinished, and the client inflated a truncated stream into empty chunks with no
error on either side. That path is what the compressBound commit closes.

@EverNife
EverNife merged commit a52dc98 into CrucibleMC:staging Sep 13, 2026
1 check passed
@KAMKEEL KAMKEEL mentioned this pull request Oct 2, 2026
EverNife added a commit that referenced this pull request Oct 3, 2026
Fix NEID Crash caused by changes of #204
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