Optimize network element and cable cluster hot paths - #1709
Open
rubensworks wants to merge 7 commits into
Open
Conversation
rubensworks
force-pushed
the
claude/performance-optimizations-276gh5
branch
from
August 26, 2026 19:47
5a219f7 to
840d6ca
Compare
CableHelpers#onCableRemoved initialized a network for every side that the removed cable was connected to. Those sides are usually still connected to each other via another path, so this walked the same cable cluster and tore down and re-derived all of its network elements up to six times for a single cable removal. Sides that already ended up in one of the networks that were just created are now skipped, so a network is only formed once per resulting cluster. Networks that genuinely split still get one initialization each.
Most PartNetworkElement operations resolved their part state twice: once directly, and once more inside getTarget(). Each resolution is a block entity capability lookup. The state is now resolved once and passed to getTarget(S), which is what the BlockState-based variants of these methods already did.
PartNetworkElement#getPriority, #getChannel and #getId each resolved their part state up to three times: once for the loaded check, once for the part presence check in hasPartState(), and once more to obtain the state itself. They now share a getPartStateOptional() helper that does this with a single part container lookup.
PartNetworkElement#compareTo dominates the cost of adding elements to and removing them from a network, since both the element set and the updateable element map are sorted. Part types are singletons, so identical part types are guaranteed to have an identical unique name and translation key. Detecting them by identity avoids both of those string comparisons. The nested comparisons are flattened into early returns to make this readable.
…lookup PartNetworkElement#compareTo already checks that both elements are loaded before comparing their priorities, so getPriority()'s own loaded check is redundant there. Both priorities are now read with a single part container lookup instead of three.
PathElementTileMultipartTicking#getReachableElements constructed a PartPos and a PartTarget for all six sides of every cable it visited, even though IPartContainer#getCapability immediately discards them when there is no part on that side. Since path finding visits every cable of a cluster, and most cables carry no parts at all, this was a significant amount of wasted allocations.
…k cube GameTestsPerformance#addCablesPostWarmup appends its cables just outside of the generated network cube, up to 100 blocks above it, while the cleanup after each measurement only clears the cube itself. Those cables were therefore never removed, so their network stayed alive for the remainder of the server run and kept being ticked while the tests that run afterwards were being measured, and it was persisted into the world save. Measured over a full run, this removes the two leftover networks of about 90 cables each that the append tests left behind (331 -> 329 persisted networks, and a 17% smaller network storage file, since those two are by far the largest ones). Note that a full run leaves several hundred smaller networks behind in total, so this does not by itself make repeated runs over the same world comparable. The appended cables are now cleared together with the cube. A bounding box variant of NetworkGenerationHelper#clearCables is added for that, so that the SKIP_NETWORK_INIT handling stays in one place.
rubensworks
force-pushed
the
claude/performance-optimizations-276gh5
branch
from
August 30, 2026 19:01
840d6ca to
837ce95
Compare
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
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.
Optimizations to the network hot paths, found by profiling the code paths that the
GameTestsPerformancebenchmarks exercise, plus one cleanup fix to those benchmarks.Every change here was individually benchmarked, and only the ones with a measurable effect were kept. Six further optimizations were tried and dropped because they measured as no-ops; they are listed at the bottom in case they are still wanted as cleanups.
Optimizations
Only re-initialize distinct networks when a cable is removed
CableHelpers#onCableRemovedcalledNetworkHelpers#initNetworkfor every side that the removed cable was connected to. Those sides are usually still connected to each other via another path, so this walked the same cable cluster and tore down and re-derived all of its network elements up to six times for a single cable removal.Sides that already ended up in one of the networks that were just created are now skipped, so a network is only formed once per resulting cluster. Networks that genuinely split still get one initialization each.
Resolve the part state only once per operation
Most
PartNetworkElementoperations resolved their part state twice: once directly, and once more insidegetTarget(). Each resolution is a block entity capability lookup. The state is now resolved once and passed togetTarget(S), which is what theBlockState-based variants of these methods already did.Cheaper part network element comparisons
PartNetworkElement#compareTodominates the cost of adding elements to and removing them from a network, since both the element set and the updateable element map are sorted.compareToalready checks that both elements are loaded before comparing their priorities, sogetPriority()'s own loaded check is redundant there. Both priorities are now read with a single part container lookup instead of three.getPriority,getChannelandgetIdshare the same single-lookup helper. This one has no measurable effect on these benchmarks (they barely exercise those methods), but it is what the priority comparison above is built on, and it is a strict 3 lookups to 1 reduction elsewhere.Skip part path elements for cable sides without a part
PathElementTileMultipartTicking#getReachableElementsconstructed aPartPosand aPartTargetfor all six sides of every cable it visited, even thoughIPartContainer#getCapabilityimmediately discards them when there is no part on that side. Path finding visits every cable of a cluster and most cables carry no parts, so this was a lot of wasted allocation.Benchmark fix
GameTestsPerformance#addCablesPostWarmupappends its cables just outside of the generated network cube, up to 100 blocks above it, while the cleanup after each measurement only clears the cube. Those cables were never removed, so their network stayed alive and kept being ticked while the tests that run afterwards were being measured, and it was persisted into the world save.They are now cleared together with the cube. Measured over a full run this removes the two leftover networks of about 90 cables each that the append tests left behind: 331 to 329 persisted networks, and a 17% smaller
integrateddynamics_Networks.dat, since those two are by far the largest ones.See the note at the bottom for the wider issue this is only a small part of.
Measurements
Method:
PERFORMANCE_BENCHMARK_ENABLED=true ./gradlew runGameTestServer, benchmarking each commit cumulatively, withruns/gameTestServer/worldwiped before every run (see the note below on why that matters). Two runs of unmodifiedmasterbracketed the sweep to establish a noise floor.Noise floor from those two identical-code runs:
redstoneioclock_removesrv ±2.6%,redstoneioclock_appendsrv ±2.7%,empty_removesrv ±3.1%,idlenet ±7.3%,redstoneioclocknet ±6.0%. The*_appendpartsserver tick rows varied by ±22-32% between identical runs and are not usable at all.Per-change effect on the metrics that are above their noise floor:
redstoneioclock_removesrvempty_removesrvredstoneioclock_appendsrvidlenetredstoneioclocknetSummarised:
onCableRemovedchange, with the comparison changes adding a further 26% on part-heavy networks.empty_*presets, which contain no parts. That null result is a useful check that the signal is real.Tried and dropped
These were implemented and benchmarked, and each measured as a no-op. I have left them out rather than carry changes that cannot be justified, but can add any of them back as plain cleanups if wanted.
Network#isValidwhen nothing is invalidated. My reasoning was that the lookup costs a logarithmic number of expensive comparisons, and that is simply wrong:TreeSet#containson an empty set returns immediately at the null root and performs zero comparisons. Note that the set is not empty for networks restored from disk, sinceafterServerLoadinvalidates all of their elements, so this may still be worth something in a world that has saved networks. It is worth nothing in these benchmarks.PathFinder. It does let the cluster be built in linear time instead of re-sorting, but it also turns every BFS insert into an O(log n) tree insert instead of an O(1) hash insert. It moves the cost rather than removing it.ResourceLocationinPartTypeBase#getUniqueName. Measured -1.7% / +1.8%, i.e. nothing, because the part type identity check above already short-circuitsgetUniqueName()in the common case.Network#deriveNetworkElementsandPathElementCable#getReachableElements, and one map lookup instead of four inCableDefault#isConnected). Together these measured -9% on theempty_*presets but +3% on the redstone ones, against a 15-17% spread between repeated runs. Not resolvable.Note on benchmark stability
Worth knowing independently of this PR: results drift upwards substantially across repeated runs that reuse the same world. Over four consecutive runs I measured
redstoneioclockserver tick rising by 425% andemptyby 179%, with the world growing by about 27MB per run.The cause is that a full run leaves several hundred Integrated Dynamics networks behind in the world save. These are persisted in
integrateddynamics_Networks.dat, and are restored and ticked on every subsequent server start whether or not their chunks are loaded, so they accumulate permanently. Each server start also places the game test grid at a completely new location in the world, so nothing is ever overwritten.The append cable fix in this PR removes 2 of those ~330 networks. The remaining ones come from the game tests at large, which is well outside the scope of this PR.
Two practical consequences:
runs/gameTestServer/worldbetween runs, otherwise the results are not comparable.Separately,
avgServerTickTimeis a mean over the server's last-100-tick ring buffer sampled at a single instant, so for the presets that do not append or remove blocks it largely reflects whatever other test batches ran nearby.avgNetworkTickTimeis much more stable and is the metric worth trusting for those presets.Testing
./gradlew buildpasses../gradlew runGameTestServerpasses, all 891 required game tests, at every commit of this branch and on master.🤖 Generated with Claude Code
https://claude.ai/code/session_01PyjctZ8hPuaU9iCKRqkWjC