Repository navigation
perf: read inventory holders without copying the block - #36
Merged
Merged
Conversation
Opening and clicking the studio menu was slow on Main because every getHolder() call on a container inventory copies the whole block state, so a click with a large chest open could stall the server. The menu and the jukebox hopper check now use getHolder(false), which returns the same holder without the copy. The code only checks the holder's type, so behaviour does not change. No call site needed the copying form. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Use non-creating holder lookups src/main/java/net/tfminecraft/musicalinstruments/studio/StudioMenu.java, src/main/java/net/tfminecraft/musicalinstruments/studio/StudioListener.java, src/test/java/net/tfminecraft/musicalinstruments/studio/StudioMenuTest.java, src/test/java/net/tfminecraft/musicalinstruments/studio/StudioListenerTest.java |
Menu and hopper holder checks now call getHolder(false). The tests stub that method for the menu inventory, jukebox, and null-holder cases. |
Estimated code review effort: 1 (Trivial) | ~5 minutes
Change: Refactor
Suggested reviewers: nowkoo
Merge Risk: 
267f6
-
The change avoids block-state snapshots without an established behavior regression or merge-blocking risk.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
github-actions
Bot
added
the
size:XS
XxFran10xX
marked this pull request as ready for review
October 31, 2026 12:42
Contributor
Author
|
@coderabbitai review |
Action performed
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
Main profiles, 20 hourly spark dumps 10-30 10:00 → 10-31 05:00 (peak 37 players).
Inventory#getHolder()without an argument builds a full BlockState copy when the inventory belongs to a block (chest, barrel, double chest), and every plugin's click/open listener calls it again. Over the 20 h that cost ~200 s of main-thread time, mostly as short stalls on clicks in heavy containers; one click at 04:01 took 2.0 s. In MusicalInstruments:StudioMenu.open20.8 s andStudioMenu.click1.6 s.Change
getHolder()calls inStudioMenuand the jukebox hopper guard →getHolder(false).For plugin menus the holder is the plugin's own object, so the result is identical. For real block inventories the code only type-checks or reads the holder, so the live state gives the same answer. Nothing players see changes.
Testing
main; Linux CI is the check.TestChar1:/faction list(row click works),/referraland/activitymenus opened and worked.🤖 Generated with Claude Code