feat(anvil): experimental multithreaded chunkloader - #91
Closed
TheMeinerLP wants to merge 36 commits into
Closed
Conversation
Adds the lower layer of the upcoming multithreaded Anvil chunk loader: sector allocation, palette bit packing, the compression schemes and the region file container itself. The container uses positional FileChannel operations so reads can run in parallel without a lock. Only the sector allocation and the eight byte header entry update are guarded, which keeps decompression and NBT work outside of any critical section. Fixes two deviations of the built-in AnvilLoader along the way: the length field now follows the specification (compression byte plus payload instead of four bytes too many) and short reads are handled instead of ignored. Chunks which exceed the 255 sector limit of a location entry are moved into an external .mcc file instead of failing. Declares adventure-nbt and jetbrains-annotations in the version catalog. Both were only present transitively through compileOnly(minestom) so far. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The getters of CompoundBinaryTag fall back to a default value when a key is missing or holds another type. For chunk data that turns a broken region file into a silently empty chunk which then overwrites the real data on the next save. The facade reports those cases as an error instead. It also avoids the array tag iterators of adventure-nbt 5.1.1 which stop one entry early. A test documents that defect so a later dependency bump makes the guard visible instead of quietly removing the reason for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PaletteData is the representation the codec uses between a region file and the palettes of Minestom. It is an immutable record so a loader thread can build it without a lock and publish it safely through the final fields. It derives the bits per entry from the length of the packed data instead of from the palette size alone. The built-in palette only looks at the palette size, which silently misreads data from a writer that used more bits than the palette requires. AnvilDiagnostics throttles repeating warnings to the first occurrence of a distinct name and collects the counters for the summary of a loader. The tracked names are capped because a broken world can hold an unbounded amount of unknown names. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Implements ChunkLoader with parallel loading and saving enabled. The work of a chunk runs in three stages so the expensive part never happens under a lock: the chunk state is copied under its read lock, the conversion between that copy and the compressed bytes runs without any lock, and only the transfer into the region file is guarded. saveChunks is overridden to group the chunks by region and to bound the concurrency with a semaphore. The default implementation starts one virtual thread per chunk which lets thousands of them compete for the same region locks while every snapshot stays on the heap. A chunk which cannot be read now fails instead of being reported as absent. An absent chunk makes the server generate a replacement which overwrites the stored data on the next save, so a read failure has to stay visible. Also collects block entities in uniform sections, which the built-in loader drops, and writes the chunk status with the capitalised key the game uses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
registerInstance wired the loader of Minestom directly into every instance. The new overload takes a ChunkLoaderFactory instead so a provider can opt into the Anvil loader of Aves. The existing overloads keep the previous behaviour, so this is additive for current consumers. The biome resolver now resolves its registry on first use instead of in the constructor. Reading it eagerly made a loader impossible to create before MinecraftServer.init has run, which also broke construction inside tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The loader is new and has not been validated against a large amount of real worlds yet. Marking the public types as experimental states that its API may still change, which matters because the surface is fairly wide. SectorAllocator stays unmarked because it is package-private and therefore not part of the public surface. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The format stores the position of a block entity in world coordinates. The loader wrote chunk local ones, which still round tripped through itself because Chunk#setBlock masks the coordinates, but produced files the game and other tools would read at the wrong position. Both directions are now consistent: saving adds the chunk offset, loading masks the stored world coordinate back into the chunk. Also bounds the amount of open region files. The loader receives unload calls for chunks it never loaded, so counting users of a file is unreliable. A limit with transparent reopening releases handles without that dependency. Sets an explicit heap for the test worker. The external chunk tests allocate payloads of about one mebibyte and the worker could die while other build tasks ran in parallel, which surfaced as an EOFException. Removes the accidentally committed .serena tool configuration and ignores it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Loading dropped the id of a block entity without resolving its handler, even though saving writes that id back. A block entity therefore lost its handler on every round trip. The id is now resolved through the block manager, the same way the built-in loader does it. unloadChunk now closes the region file once the last chunk this loader read from it has been unloaded. Only chunks this loader handled itself are tracked, because a loader also receives unload calls for chunks it never loaded; such calls are ignored instead of closing a file still in use. The open file limit stays as a backstop for chunks that are never unloaded. Documents the performance and memory differences with the built-in loader, separating structural facts from the one-off measurements taken while designing it, and states explicitly where this loader is not faster. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Records the findings from the exception hierarchy, InstanceContainer and light engine investigations. Each answers a question that cost real effort and will be asked again: can this part of Minestom be replaced, and is it worth it. Claims are tied to the sources of Minestom 2026.06.20-26.1.2 or to probe code compiled against that jar. Measurements are named as such. Where the agents disagreed, both positions are recorded instead of silently resolved. Two of the findings apply to code that already exists here: Section.clone falls back to Minestom's light implementation, which the save snapshot uses, and the light array length check of the built-in loader is still missing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TheMeinerLP
force-pushed
the
feat/aves-anvil-chunk-loader
branch
from
July 30, 2026 23:20
2a0a9ee to
633ec87
Compare
Computes the light of a section from the blocks it holds, resolving the properties of a block once per distinct state instead of on every visit. A breadth-first search reaches a block from up to six directions, and walking palette, registry and occlusion shape each time is the dominant cost of the naive form. Memory follows the same idea. A section whose levels are all equal carries no array at all, which is the common case rather than an edge case, and the propagator reuses its level buffer and queue across runs so repeated propagation allocates nothing beyond its result. Occlusion is stored per face. Roughly one in seven block types occludes some faces and not others, slabs and stairs among them, so a single flag per block would be wrong for a large amount of real blocks. Only the entered face decides whether light passes. Testing the face it leaves as well would keep every emitting block that is opaque itself dark, and glowstone is exactly that. The propagation has no Minestom dependency; the registry sits behind BlockLightSource, the same separation the Anvil codec uses for palettes. The adapter maps faces by ordinal, which a test pins against the server enum. Documents explicitly that this does not run when loading pre-lit worlds, and which parts of a full light system are still missing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds the two pieces the engine needed to be usable rather than only correct. ChunkLightPropagator runs the search across all sections of a chunk at once. A propagation that stops at a section border leaves a visible seam every sixteen blocks, because a source near the border lights its own section and nothing beyond it. ChunkLightService connects the engine to a server. It reads the block states of a chunk, propagates without holding a lock, and hands the result to the sections through Light#set. That method is not marked internal, unlike the calculation methods of the interface, so the service does not implement Light itself and cannot break when those signatures change. Writing through set also clears the update flag, so the server does not recompute the result. This works with any chunk of any loader; a test covers the round trip through the Anvil loader of Aves explicitly. Locking follows the split the Anvil loader uses: states are read under the read lock, the propagation runs with no lock held, and only the transfer of the result takes the write lock. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes the three gaps the engine was documented as having. Sky light enters from above and falls without losing a level until something stops it, which is what makes an open field lit at every height and a cave dark. Only after the fall is interrupted does it spread like any other light. It shares the search with the block light and differs only in its seeding. Light now crosses chunk borders. A chunk lit on its own ends its light at the edge, which shows up as a straight dark line every sixteen blocks; calculateWithNeighbours exchanges the border levels with every loaded neighbour in both directions and skips the ones that are absent. ChunkLightState updates an existing result after a single block changed. Adding brightness only spreads, but removing it needs the light that was already stored around the source retracted first, or the glow stays without its source. The update therefore retracts every level that originated at the changed position, collects the still valid levels at the edge of that area, and spreads those back in. A test asserts the incremental result is identical to a full recalculation, block for block. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The queue of both propagators was sized for one entry per position, which assumed a position is queued at most once. That is wrong: a position is queued again whenever a brighter source raises its level, so sources of different brightness overflow it. A test reproduces the exception, and the queue now grows instead of relying on the assumption. The same sizing in ChunkLightState is guarded as well; it could not be made to overflow, but its bound was no more proven than the other one. The border exchange now repeats over the loaded neighbourhood until a sweep raises nothing, so light no longer stops one chunk short. Injection reports whether it raised a level, which is the convergence signal, and a hard cap keeps a pathological case from looping. Sky light updates keep the highest blocking position per column, so a change rescans only that column instead of re-seeding all of them. Both directions are covered: placing a block lowers the sky below it, removing one re-opens the column. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The stress tests assert invariants rather than the absence of exceptions, and each was verified against a deliberately broken copy of the source to prove it actually catches the corruption: removing the region lock is detected in 6 of 6 runs, making the propagation buffers static in 3 of 3. They found two real defects, both fixed here. AvesAnvilLoader could evict a region file between the moment a thread obtained the handle and the moment it wrote, losing that chunk; the write now retries once with a fresh handle. The name cap of AnvilDiagnostics was a check-then-act that racing threads could push past, so an overshooting insertion is taken back. The benchmarks live in their own source set, so JMH never reaches the classpath of the published library and no benchmark runs during a normal build. They turn the central design claim of the loader into a number: of a chunk save, the snapshot holds the chunk read lock for 76 us and the transfer holds the region lock for 14 us, while the 3016 us of codec work run under no lock at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
theEvilReaper
changed the title
Compression is the largest single cost of saving a chunk. Measured over the stages of a save, the snapshot under the chunk read lock takes 64 us and the transfer under the region lock 17 us, while compression alone accounts for roughly 2700 of the 4270 us the whole operation costs. On a serialised 24 section chunk, the platform default spends about eighty percent longer than level two to produce a result about three percent smaller, and the highest level costs half again as much for a further half percent. Level two is therefore the default now, and the level is a parameter so a world that is written once and read many times can ask for a smaller result instead. A first attempt used level four, which measurements on real chunk data showed to be pointless: it costs nearly as much as the platform default while saving almost nothing over it. That earlier figure came from a payload of random bytes, which compresses nothing like chunk data does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Most sections of a world hold one repeated state: air above the terrain, stone below it, water in an ocean. Both the palette encoder and the opacity table walked all 4096 entries through a map for those, only to collapse the result again afterwards. A uniformity scan now runs first and stops at the first differing block, so it costs nothing for every other section. Encoding a uniform section drops from 27.9 to 0.54 microseconds, and building its opacity table from 40.8 to 0.54, or to 0.99 when resolving a state is expensive. The opacity table of a uniform section also stops allocating its two 4096 byte arrays entirely. This does not change the comparison against the built-in engine. Those sections always carry a light source next to air and are therefore never uniform, so the shortcut cannot apply there. It pays off on the sections a real world is mostly made of, not on the ones that benchmark measures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collects what was built, what was measured, what is open and what was deliberately not built, so the branch can be picked up without reading sixteen commits. Also drops a limitation from the loader document that no longer applies: block entity coordinates were fixed to world coordinates a while ago, but the entry describing the old behaviour stayed behind. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The file described the outcome but assumed the reader already knew the subject. It now starts from what this is and why, names the versions and commands, and states the conventions the build enforces. The largest addition is a section of facts that each cost real effort to establish and are invisible in the code that resulted from them: that loadChunk is synchronous and parallelism comes from the caller, that Palette is sealed while Light is not, that the adventure-nbt array iterators skip their last element, that a registry lookup in a constructor makes a type untestable. Someone picking this up without them would rediscover them the same slow way. Also records the decisions that shape the rest, a file map with a reading order, and links to the published charts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The service kept a ChunkLightPropagator in a field, so two threads sharing one service shared its scratch buffers. That produced wrong light in nearly every concurrent call, and because the result is handed over through Light#set, which clears the update flag of the section, the server never recomputed it. The world simply carried wrong light. ChunkLightState already built a propagator per call, which is why calculateWithNeighbours was never affected; calculate and calculateSky now do the same. ThreadLocal, volatile and synchronized were all rejected: the first is costly under virtual threads, the second removes the exceptions and leaves the silent corruption, and the third serialises exactly the parallelism the type exists for. The class documentation said nothing about thread safety while calculate suggested the opposite, so the contract is now stated explicitly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The class promised that a read needs no lock because positional channel operations are safe from several threads. That is true of the channel but not of the region file: a chunk which is rewritten releases its sector range, the allocator hands that range to the very next write of any chunk, and a reader which is somewhere inside it then parses foreign bytes. It either rejects the length field or, worse, returns a payload which looks plausible and fails much later. The two tables were plain int arrays on top of that, written under the lock and read without one, so a reader was also free to observe a stale zero, which means "no such chunk" and makes the server generate the chunk again and overwrite the saved one. Every one of the 1024 entries now carries a version counter which a writer raises on entry to and on exit from its critical section. A reader takes the counter, refuses an odd one, reads the bytes and takes the counter again; only an unchanged even counter is accepted. A range can only be recycled after the chunk which owned it was rewritten, so an unchanged counter really does prove that the bytes belong to the version the reader started with. After four attempts the reader falls back to the writer lock so a chunk which is rewritten in a loop cannot starve it. The tables became AtomicIntegerArray, which removes the stale read as well. The counter has to be raised on both ends. Raising it only at the end leaves a window in which a change is already visible on disk while the counter still shows the old value, and a reader can fit entirely into that window. That is not theoretical: it is what the storage switch test kept catching after the first attempt. The same read also had no way to follow an oversized chunk. The header entry decides whether the payload lives in the region file or in the .mcc file next to it, and that entry was guarded while the file operations were not: the file was written before the lock and removed after it. Two writers of the same chunk, one oversized and one not, could therefore leave a header which claims an external payload behind a file the other one had already deleted, and a reader ran into NoSuchFileException. A reader could also read the .mcc while Files.write was still filling it, because that write truncates first. Both file operations moved into the critical section which rewrites the entry, and the payload of an oversized chunk now goes into a staging file next to the region file and is moved into place with an atomic rename while the lock is held. The bytes therefore still leave the process outside the critical section, the entry and the file always agree, and the file appears in one step instead of growing under a reader. The order inside the section is deliberate as well: the file appears before the entry points at it and is removed after the entry stopped pointing at it, so a crash in between can leave an unused file but never a missing one. Two alternatives were rejected. A ReadWriteLock would make every read take an atomic and, worse, would block every reader of the region for the whole duration of a payload write, which is the one thing this loader gains over the Minestom AnvilLoader; about 97 percent of a save deliberately runs outside any lock today. A deferred free would need reader registration on a shared counter, which puts the contention back that the design avoids, and it would delay sector reuse and grow the files. A per entry counter costs 4 KiB per region file and touches nothing shared between chunks. Cost: two volatile reads per read and two atomic increments per write. RegionFileBenchmark shows no difference outside the run to run noise of the machine (readRaw 17.7-19.8 us/op before, 17.1-19.7 us/op after). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A region file handle could be closed while another thread was reading through it. `loadChunk` obtained the handle, decoded the chunk and only then registered it in `trackedChunks`, so a parallel `unloadChunk` could drop the last tracked chunk of that region and close the channel under a reader. `evictRegions` did the same to every handle except the one just opened, as soon as more than `openRegionLimit` regions were touched. The load then failed with a `ClosedChannelException` even though the file on disk was perfectly intact, and that failure propagates as an `AnvilChunkException` which stops the chunk from loading at all. The write path already knew about this race and worked around it with a retry plus `RegionFile.isClosed()`. A retry only treats the symptom: the window stays open, the failure merely becomes less likely, and the read path would need the same workaround. The cause is that a handle can be closed while it is in use, because the loader registers a chunk only after it has been read. Reference counting the chunks instead is not an option here — Minestom documents `unloadChunk` as reachable for chunks the loader never loaded, which is exactly why the current design tracks only its own chunks. Every access now registers itself on the cached handle before it touches the file and deregisters afterwards. Dropping a handle from the cache and closing it become two separate steps: an unload, an eviction or `close()` only removes it from the cache, and whichever thread leaves it last performs the close. A handle which was dropped refuses further users, so a thread that finds one opens the file again instead of receiving a channel that is about to be closed. The retry in `writeToRegion` is therefore gone, and `RegionFile.isClosed()` has no caller left. The cap keeps bounding the number of cached files exactly. The number of open descriptors can exceed it while an access is running, which is the price for never breaking work that has already started. The two new tests reproduced the failure reliably before the change, with 80 and 15 failed loads out of 480 attempts and `IOException: ... is already closed` and `ClosedChannelException` as the causes. The test for the open file limit no longer tolerates failed saves, because an eviction can no longer break one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`closed` was written and read in `close()` and nowhere else. Because the loader reports parallel loading and saving as supported, the server runs one virtual thread per chunk, and those tasks are still in flight when the shutdown closes the loader. A task which reached the region cache after that opened a fresh `RegionFile`, published it into the map that `close()` had just emptied and left it there: a file descriptor nothing closes again, plus writes into a world which is already considered closed. The flag was an invariant nobody enforced. Three behaviours were possible: ignore the call, refuse it, or have `close()` wait for the running tasks. Ignoring is out for a load, because `null` means "chunk absent" to Minestom and makes the server generate a replacement which overwrites the stored chunk on the next save; ignoring a save would drop chunk data during the very shutdown it belongs to. Waiting is out because the loader does not own the load tasks — Minestom starts them — so it has nothing to wait on. What is left is refusing: `loadChunk`, `saveChunk` and `saveChunks` now throw an `IllegalStateException`, which is loud, is never `null`, and names a caller lifecycle error rather than a broken chunk. The window between the check and the publication of a handle is closed with the flag itself: `close()` raises it before it empties the cache, and the opening thread reads it after publishing. Whichever of the two loses the race sees the work of the other, so a handle can no longer survive the shutdown. `close()` is synchronized so a concurrent second call cannot slip past the flag, and a file which a task is still using is handed to that task to close, which is what the registration introduced in the previous commit makes possible. The refusal never masquerades as a data error. In `loadChunk` the region acquisition sits outside of the reporting block, and `saveChunk` rethrows the refusal before its own catch counts it: a shutdown is neither an error of the chunk nor something the exception manager should see twice. `testClosingWhileLoadsAreRunningLeavesNoOpenRegionFile` failed before the change with `AnvilChunkException: The chunk 64/0 could not be loaded`; the three direct tests failed with "Expected IllegalStateException to be thrown, but nothing was thrown". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
isClosed existed for one purpose: writeToRegion obtained a handle, found it closed because another thread had evicted it, and retried. That retry is gone now that a handle carries a usage count and is only closed by its last user, so nothing in the loader, the tests or the benchmarks called the method any more. The field it read stays; close and ensureOpen still need it. What goes is the public getter, whose documentation described a recovery procedure that no longer exists and would have misled the next reader into rebuilding it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The status file claimed 552 tests, 17 commits and one open thread-safety defect. All three are stale: the light service is fixed, four more races were found by searching the rest of the code for its shape, and the count is now 563 tests over 23 commits. What replaces the old first open item is the part that is genuinely still open: one service now serves many threads, but two threads lighting overlapping neighbourhoods still resolve last-writer-wins, and two of the guarantees added here are argued from construction rather than covered by a test. Both are written down as such so the next reader does not mistake them for settled. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…hips The comparison so far was structural: the source of both implementations shows where the work sits relative to the lock, but nothing said what that is worth. Minestom's AnvilLoader cannot be reached from a benchmark fork, because its static fields read the biome registry and the block state count and the class initialiser fails before any measurement starts. Its RegionFile reads no registry, so the comparison happens one layer down, where the lock difference actually lives. That benchmark sits in the Minestom anvil package because RegionFile is package-private there, the same arrangement the light engine comparison already uses. Fairness of the compression is enforced rather than assumed: both sides run the identical Adventure writer over byte-identical payloads, so the save comparison measures the loaders instead of the zlib level. The level itself is measured separately. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The comparison table had twenty equally weighted rows and no way to tell which of them change what a server does. It is now grouped by consequence — silent data loss, a failure that destroys the original, concurrency, and the rest — and the concurrency group carries diagrams, because lock granularity is the one thing prose explains badly and a picture explains at once. Five diagrams: which steps sit inside the region lock on either side, what two threads reading one region file actually do, the write lock spanning the whole serialisation against a read lock spanning a clone, and the Phaser branch whose missing arriveAndDeregister hangs the saving thread for good. The section that claimed no benchmark suite ships with this loader is corrected and now separates structural claims from measured ones. Both measurements are reported with the parts that do not flatter this loader: on a single thread Minestom is marginally ahead, and the save path differs by 1.14x to 1.16x with overlapping spreads everywhere else. The read path under contention is the one large effect, and even there the magnitude exceeds what plain serialisation predicts, which is written down as unexplained rather than quoted as a win. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…isons The two comparison sections of anvil-chunk-loader.md carried numbers in tables only, and light-engine.md had no comparison against the built-in engine at all. Both now show the measurements as mermaid line charts, each with a plain-language reason underneath. anvil-chunk-loader.md: - concurrent readers, shown twice: both loaders on a shared scale, then Aves alone on a scale that fits it, because a common scale flattens it - saving a chunk, where the finding is that the lines nearly coincide light-engine.md gains a whole comparison section: the benchmark, the measured table, one chart per occlusion setting, a flowchart explaining why the opacity table loses on empty sections and wins on full ones, a flowchart of the two ways light reaches a chunk, and a flowchart of the retract-then-refill update. It states plainly where Minestom is ahead, that Minestom is the steadier of the two, that its light engine has no concurrency problem to fix, and when to prefer it. xychart-beta draws no legend and no error bars, so both are carried by the text: every chart names its lines, and every point whose spread swallows the difference is called out as not established. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…er claim The file opened with "None of the benchmarks start a server", which stopped being true when the head-to-head benchmarks arrived: two of them have to start one, because the Minestom types they measure read registries in static fields and would fail their class initialiser otherwise. Four benchmarks were missing from the list entirely, two of them from before this branch. They are now grouped by what they compare against, because the distinction decides how a number may be read: a comparison figure includes registry time on both sides and must never be held against a library figure, which deliberately excludes it. ScalingBenchmark gets its own section rather than joining the comparisons — it measures this library against itself, and its point is the fifteen-step parameter sweep that shows where sky light stops being linear. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…as it open The external file of an oversized chunk is the only place where the lock free reads of a region file meet a name in the file system instead of a range inside the region file, and a name is not a POSIX concept. A reader opens the file through Files.readAllBytes while a writer replaces or removes it, which POSIX allows because a deletion detaches the name at once and keeps the unnamed file alive for every open handle. Windows keeps the name in the directory and only marks the file for deletion, so for as long as a reader holds it open every attempt to open that name or to move another file onto it is denied. Two writers which switch the same chunk between the two storage locations therefore denied each other on Windows: the one which dropped the external file poisoned the name for the one which wanted to put a new file there. The removal now renames the file onto a private name before it deletes it, which detaches the name at once on both systems and leaves only the private name in the pending state where nobody asks for it again. What Windows can still deny for a moment on its own, a handle which is being torn down or a virus scanner which opened the file behind our back, is repeated for a bounded time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A test that fails on one operating system only is diagnosed from the console summary today, and that summary names the test and the exception type but not the message, the path or the stack trace. The Windows failure this week had to be reasoned out from the platform semantics instead of read off the report, because the report was gone the moment the job ended. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The research asked whether an algorithm from another engine would close the gap to Minestom. It would not: the BFS already pushes levels onto neighbours instead of pulling from all six, which is the advantage attributed to Starlight. The gap sits in the preparation and the collection around the search. Adds the refuted leads (Starlight's border arrays, its data-holding gain, the 12x/28x/37x figures, Phosphor, the absent literature, the Vector API packaging trap), the rebuild measurements with their provenance marked, and the correctness finding that calculateWithNeighbours writes back eight ring chunks it only lit from inside the 3x3. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A stage benchmark of both engines shows where the time of a section goes.
Building the opacity table cost a flat 31.4 us regardless of how many
sources or solid blocks the section held, which is 41 percent of the
scenario Aves lost hardest, a single source in an empty section.
The table allocated 74040 bytes per section while the two arrays it keeps
are 8224. The difference is 4096 times 16 bytes, one throwaway object per
block: the lambda handed to computeIfAbsent captures the source, so it is
capturing and is created inside the loop, and computeIfAbsent is too large
to inline for escape analysis to remove it again. That garbage is also why
Aves scattered by 19.1 us where the built-in engine scattered by 2.7.
The map is replaced by a linear probing table over the raw state id, which
neither boxes a key nor creates a value object, and which dies with the
build that created it. A run of one repeated state is answered from the
previous block instead of the table, because blocks of a world come in
runs. Packing a resolved state into a short lets a single array carry both
the occluded faces and the emission.
Collecting a result wrote every one of the 4096 positions through
LightNibbles.set, which range checks and reads the byte a level shares with
its neighbour every time. LightNibbles.ofLevels packs two neighbours at
once and checks the whole range with one comparison at the end.
Measured on the same machine in one session, before against after, with
the built-in engine as the control:
sources solid before after built-in
1 0% 74.2 +- 1.9 44.5 +- 0.6 49.4 +- 1.3
1 30% 61.8 +- 1.6 39.3 +- 0.8 62.0 +- 2.0
8 0% 137.7 +- 7.4 98.3 +- 2.4 121.1 +- 5.5
8 30% 144.7 +- 1.8 119.3 +- 3.5 204.2 +- 3.7
64 0% 135.9 +- 2.8 109.2 +- 1.6 126.5 +- 5.6
64 30% 152.7 +- 1.8 122.6 +- 1.3 206.6 +- 4.2
Aves now wins all six scenarios instead of two, and the lead at 30 percent
solid blocks grew from 1.31x to 1.68x rather than being traded away. The
table allocates 8664 bytes instead of 74040.
The engines are byte identical over all 54 scenarios, which no test had
pinned down until now, and one service still serves many threads: nothing
here is shared, the cache is local to a single build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The comparison benchmark only ever placed glowstone, so every source in every scenario emitted level 15. The propagation of Aves is ordered by that assumption, and a change that trades the ordered queue for a bucket queue would show up as a regression on this input while winning on the input real worlds have. The new emissionMix parameter adds the second case. MIXED cycles the sources through glowstone, lantern, torch, redstone torch and magma block, which the registry gives 15, 15, 14, 7 and 3. The block is picked by the placement round rather than by another draw from the seeded random, so both mixtures place their sources at the same positions and the levels are the only difference between them. UNIFORM keeps the old glowstone only form down to the drawn positions, so the six documented scenarios stay comparable. Every trial now also verifies that both engines return the same 2048 bytes before a measurement is taken. The documentation already claimed the benchmark did this; it did not. Mixed levels are the input on which an order dependent search would drift, and all twelve scenarios agree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every Aves reference in the anvil document carried a file and a line number. The concurrency work of this week grew AvesAnvilLoader from about 730 to 1135 lines and RegionFile along with it, so those numbers now point at blank lines, javadoc fragments and stray */. The document claimed a precision it did not have, and following a reference led nowhere. Correcting the numbers would only postpone the problem: a line number into the sources of the branch that is being worked on is wrong again with the next commit. Aves references therefore name the member instead — RegionFile#writeRaw for a method, RegionFile.OPTIMISTIC_ATTEMPTS for a field, a bare type name for the type. Every one of them was looked up in the current sources. Minestom references keep their line numbers. That version is pinned, the files do not move again, and the line is the most precise pointer available. The comparison section now says why the two sides are cited differently so the asymmetry reads as intent rather than as sloppiness. Two statements had to follow the code rather than only the reference. The time table claimed the region lock covers a positional read on the Aves side; since the per entry seqlock the read takes no lock at all and only falls back to it after four failed attempts. The comparison table claimed that out of range sections report once per loader lifetime through the diagnostics; AnvilDiagnostics#reportSectionOutOfRange has in fact never had a caller in the loader, which logs such a section at TRACE without throttling. The claim was wrong from the start, not made wrong this week. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The comparison against the built-in engine, the stage breakdown and the list of what is still open all described the state before the opacity table was rewritten. Replaced with the run of 69381af, which puts Aves ahead in all six scenarios instead of two, and with the stage figures that show where the difference came from: the search was already the faster of the two, the deficit sat entirely in the preparation. The mixed brightness rows of 0e8fbb5 are new, and they retire the reason the bucket queue was left undecided: the benchmark could not produce the case the queue wins, and now it can. Two rebuild estimates are now real JMH measurements and say so; the ones that are still estimates keep the marker they had. The claim that the engines are byte identical rested on nothing until 69381af added a test for it, which belongs in the defect list rather than being quietly fixed. The Windows defect of 78e196c is recorded with the note that it is older than this branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every measured number in light-engine.md predates 69381af, which took the per block allocation out of the opacity table. The tables and both line charts showed an engine that lost four of six scenarios; it now wins all six, so the charts also had Aves on the wrong side of their titles. Replaced the comparison table with the run of that commit, redrew both xychart-beta blocks and corrected the "upper"/"lower" hints in their titles. The section that explained why Aves loses on sparsely occupied sections keeps its mechanism, fixed preparation cost against variable query cost, because the mechanism is what makes the optimisation worth that much; only the conclusion moved from a loss to the narrowest of the six wins. The flowchart on the turning point says the same: the break even point sits below the sparsest measured section now instead of inside the measured range. Added the stage breakdown of LightEngineStageBenchmark, which is where the 31.3 us of the table build came from, and with it the correction that the Aves search was already faster than the built in one before the change. The deficit was preparation, not propagation. MIXED is documented as its own table rather than a third curve, since xychart-beta draws no legend and the mixture exceeds the shared y scale. The finding that mixed brightness costs Aves about a third at 64 sources, and why the breadth first search is what pays it, is spelled out there. The section on when to use the built in engine is restated rather than deleted. Its reason is gone but the advice partly survives: Minestom is within a few percent on open sections with mixed sources and costs no extra call site. benchmarks.md gains emissionMix, LightEngineStageBenchmark, the two recommended command lines and a corrected configuration count. anvil-chunk-loader.md gets 13 test classes with 178 tests instead of nine with 143, and 575 for the project. The byte identity over 54 scenarios was asserted in all of this without a single check behind it. LightEngineEquivalenceTest and the benchmark setup now verify it, and the documents say so instead of claiming it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
https://github.com/OneLiteFeatherNET/Falco is now everything around world loading and saving |
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
1 participant
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
Adds an experimental Anvil chunk loader as an opt-in alternative to the loader Minestom ships with. It reads and writes the same region files, stays compatible with
ChunkLoader, and uses Adventure NBT directly — Minestom already speaksCompoundBinaryTag, so no conversion layer is involved.Every public type is marked
@ApiStatus.Experimental. Nothing changes for existing consumers:AbstractMapProvider.registerInstancekeeps the built-in loader unless aChunkLoaderFactoryis passed explicitly.Why
Minestom's
AnvilLoaderreportssupportsParallelLoading() == true, but itsRegionFileserialises reading, decompression and NBT parsing through a singleReentrantLock(RegionFile.java:42). The parallelism is therefore mostly nominal. The gain is not in starting more threads but in moving the CPU work out of the lock.Design
Three stages, strictly separated:
CompoundBinaryTag↔ snapshot, zlib, bit packingFileChannelread/write plus an 8-byte header patchFileChannelpositional operations do not touch the channel position, so reads run in parallel without a lock. Only sector allocation and the header entry are guarded.saveChunksis overridden: chunks are grouped per region file, one task per region, bounded by a semaphore. The default implementation starts one virtual thread per chunk, which lets thousands compete for the same region locks while every snapshot stays on the heap.Behaviour differences worth reviewing
catch (Exception) → return null(AnvilLoader.java:117-120) — the server treats the chunk as absent, regenerates it and overwrites the real data on the next saveAnvilChunkExceptionso the failure stays visible5 + N; the format specifies1 + N(RegionFile.java:31,99,123)1 + Nstatus(AnvilLoader.java:133,396); the game writesStatusStatus, accepts both when readingfile.read(data)ignored (RegionFile.java:85)Objects.requireNonNull→ NPE → whole chunk discarded (AnvilLoader.java:263)IllegalStateException, chunk not saved.mccfileAnvilLoader.java:436-470)PaletteImpl.java:127-132)Scope
Deliberately not included:
entities/andpoi/regions, DataFixer/DataVersion migration, LZ4 (type 4) and custom (type 127) compression, corruption recovery,level.dathandling. Unsupported compression fails with an explicit error instead of misreading data.Logging
Leaf classes (
BitPacker,SectorAllocator,NbtReads,PaletteData,SectionCodec,ChunkCompression) carry no logger and only throw — they have no chunk or region context to report. The loader adds that context and owns the reporting, so each failure is logged exactly once. Repeating warnings are throttled to the first occurrence per distinct name viaAnvilDiagnosticsand capped, and a summary line is written on close.Tests
Developed test-first. 400 tests pass across the suite.
AvesAnvilLoaderIntegrationTestruns against a real environment via Cyano'sMicrotusExtensionand covers the full round trip: blocks, block NBT, block properties, parallel save and load, diagnostics counters, and that a corrupted chunk fails rather than looking absentNbtReadsTestdocuments a defect in adventure-nbt 5.1.1 as an executable check: the array tag iterators stop one entry early, which would silently drop the last entry of every packed block arrayBuild
adventure-nbtandjetbrains-annotationsare now declared in the version catalog. Both were only on the classpath transitively throughcompileOnly(minestom), so any direct use compiled by coincidence.Follow-ups (not in this PR)
AbstractMapProvidersets the loader viasetChunkLoaderafter construction, soChunkLoader#loadInstanceis never invoked andlevel.datis never read. This predates this PR.