[pull] 1.21.4 from cabaletta:1.21.4 - #25
Merged
Merged
Conversation
With Baritone installed the client does not exit when the window closes: it sits in shutdown until Mojang's ClientShutdownWatchdog kills it, about fifteen seconds later, and on Windows the leftover javaw holds a lock on the mods folder until then. The shared pool is built with the default thread factory, so every worker is non-daemon, and none of them ever exit. The core threads (corePoolSize 4, and allowCoreThreadTimeOut is never set) park in take() for the life of the JVM whether or not they have work, and two of them are permanently busy anyway with CachedWorld's chunk packer and its ten-minute save loop. Once Minecraft.main returns, those are the only non-daemon threads left. Daemon threads fix the exit. They also mean a periodic cache save that is mid-write at exit can be cut off; #5116, which predates this PR, addresses that with a shutdown hook. The threads are named Baritone-N so a dump reads as such rather than pool-4-thread-N. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
sameBlockstate compares each property's value with !=. The values come out of BlockState.getValues() as boxed Comparable, so this is reference identity. It is correct for enum and Boolean properties, whose values are singletons, and for an IntegerProperty it happens to work today because every vanilla integer property stays inside the -128..127 range the JVM's Integer cache interns (the largest, age, tops out at 25). Nothing in the comparison guarantees that, and identity is not what is meant here; equality is. Objects.equals to match the three other comparisons in this file. Plain equals would also be correct: both states share one StateDefinition, so the lookups cannot return null. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
world.dimensionType() goes through a Holder every call and was being hit twice per block lookup in BlockStateInterface.get0, plus once per node in the A* loop. Cache minY/maxY/height once at construction instead. Also split CalculationContext so everything that needs a live player or client world is computed up front and passed to a protected constructor. That lets a context (and a BlockStateInterface subclass) be built without a running client, which is what the offline pathing benchmark needs. No behaviour change; the y bound check in A* now uses the real maxY instead of the dimension height.
Four changes to the search loop itself, none of which alter which path comes out: - Nodes now live in 16x8x16 cells (a PathNode[2048] per cell, indexed by the low coordinate bits) and only the cells are in a hash map. The flat position -> node map was the single hottest thing in the profile, since every lookup was a cache miss into a multi megabyte key array. Successors are within a few blocks of the node being expanded so most land in the cell we just used. - The heap keeps combinedCost in a double[] next to the node array, so sift-up and sift-down compare primitives instead of chasing a pointer per level. - Favoring is looked up once per node when it's first reached and cached on the PathNode, instead of once per incoming edge. The multiplier only depends on the destination so this is exactly equivalent. - The seven partial-path coefficients are multiplied by precomputed reciprocals instead of divided.
The cost functions read roughly 150 blocks and evaluate about 30 mining durations per expanded node, and most of that is the same handful of blocks over and over. This makes each of those cheaper or rarer: - canPlaceAgainst went through Block.isShapeFullBlock, a guava LoadingCache lookup that allocates a queue node on every hit. It was ~8% of the search (and ~45% of all allocation) just to check whether air is a full cube. It's now a bit in the precomputed flags, as is the block-only half of avoidBreaking, which was an ArrayList scan. - PrecomputedData is shared across searches and only rebuilt when one of the settings it depends on changes, instead of starting cold (and allocating 25kB) every tick. - The flags are stored on the BlockState itself through a mixin field, stamped with the table's generation, so a predicate is one field load rather than a Reference2IntOpenHashMap lookup. Falls back to a small identity cache and then the registry when the mixin isn't applied. - The pathing thread's BlockStateInterface has a 64k entry direct mapped position -> state cache in front of the chunk/section/palette chain (~80% hit rate on the benchmark). - getMiningDurationTicks is memoized by position for the search thread's context; the block under a node was being evaluated from four descends, a downward and the neighbours' descends, each reading five neighbours for the avoidBreaking check. Together with the previous commit this is 1.2x to 1.6x more nodes per second on an offline benchmark, depending on terrain.
…reads Let the game exit: daemon threads for the shared pool, and wait for the final cache save
…uality Compare block state property values with equals, not reference identity
the identity cache was only there for running the pathfinder outside the game (benchmarks), where the mixin can't apply. that case just goes through the registry array now, which is what it used to do anyway.
Make the pathfinder consider more movements per second
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )