Async-read view for LevelChunkSection#733
Open
duranaaron wants to merge 2 commits into
Open
Conversation
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.
Why
Plugins that read blocks off-thread currently have to call
LevelChunkSection.copy()andread through the returned wrapper. That works, but you end up with a full
LevelChunkSectionincludingnonEmptyBlockCount/tickingBlockCountfields that gostale as soon as a main-thread write happens, plus an object you have to keep around for
the duration of the read.
What you actually need is a stable
PalettedContainer.Datareference for the block stateand biome containers, nothing else.
This adds a small API for that.
LevelChunkSection.acquireAsyncReadView()returns anAutoCloseableview backed by snapshotDatainstances for both containers.How it works
acquireAsyncSnapshotclones the currentData, publishes the clone as the new livecontainer, and returns the previous instance to the caller. That instance is no longer
referenced by the container and is treated as read-only.
All writes after that point, synchronised or unchecked, go to the new live instance. The
snapshot held by the caller is never mutated.
The write path is unchanged. No read counters, no copy-on-write branches, no interaction
with
getAndSetUnchecked.Benchmarks
Each acquire does one
Dataclone, same asLevelChunkSection.copy(). Most of the costin
copy()already comes fromBitStorage.copy()andPalette.copy(), so this does notreduce the raw cloning cost.
The benefit is in usage. Callers no longer deal with a full
LevelChunkSectionthat hasinternally inconsistent state, and the API leaves room for improving how snapshots are
backed later without changing call sites.
Notes
releaseAsyncSnapshotis currently a no-op. It exists so a future implementation cantrack readers without changing the API.
setBlockState, so any value here would be misleading.Context
I don’t usually work on server forks, but I ran into this while building a plugin that
does a lot of off-thread block sampling. This approach worked well for that use case, so
I figured it was worth sharing.