Repository navigation
perf: let item scan handlers skip slots whose item has not changed - #50
Conversation
The scan visits one player every 2 ticks and runs every handler on every slot of their inventory and open container, even when nothing in the slot changed since the last pass. On Main that was most of the scan's cost. Handlers now get a scan() method that does the work and returns when the same item needs the handler again: EVERY_SCAN, WHEN_CHANGED, or a wall-clock time (for food that ages). The scan remembers a copy of each slot's item per player (their own inventory and the open container) and skips a handler while the slot still holds an equal item and its time has not come. A slot whose item a handler changed is forgotten, so every handler sees the new item on the next scan, as before. The default scan() calls matches and then update and returns EVERY_SCAN, so handlers that don't override it (Magic's) see every slot on every scan, exactly as now. rescanAll() forgets everything, for plugins that reload config. Memory is dropped on quit, when handlers change, when the server is empty, and when the player closes a container. Empty-slot checks use isEmpty() instead of getType(). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The default ItemScanHandler.scan returned EVERY_SCAN for every slot, so a handler that doesn't override scan (Magic's artifact care) made every non-empty slot run, and be copied for the change check, on every scan. matches only sees the item, so an item it doesn't match is now skipped until the item changes; matched items still run every scan. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 41 minutes. View limit details
|
| Layer / File(s) | Summary |
|---|---|
Handler scan contract src/main/java/net/tfminecraft/tlibs/itemscan/ItemScanHandler.java, README.md, src/test/java/net/tfminecraft/tlibs/itemscan/ItemScanServiceTest.java |
The interface adds EVERY_SCAN, WHEN_CHANGED, and a default scan method. The README describes deferring scans for unchanged items. Tests configure handlers to use the default method. |
Per-player slot scheduling src/main/java/net/tfminecraft/tlibs/itemscan/ItemScanService.java, src/test/java/net/tfminecraft/tlibs/itemscan/ItemScanServiceTest.java |
The service stores per-player inventory and slot state, then runs handlers when their due times arrive. Tests cover scheduled scans, item changes, and container inventory replacement and reopening. |
Scan lifecycle and invalidation src/main/java/net/tfminecraft/tlibs/itemscan/ItemScanService.java, src/test/java/net/tfminecraft/tlibs/itemscan/ItemScanServiceTest.java |
The service clears cached state when handlers change, players leave, or scanning stops. It distinguishes player inventories from open containers and scans picked-up items directly. Tests cover rescans, quit events, handler changes, and empty handler lists. |
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Feature
Sequence Diagram(s)
sequenceDiagram
participant PlayerInventory
participant ItemScanService
participant SlotMemory
participant ItemScanHandler
PlayerInventory->>ItemScanService: provide slot item
ItemScanService->>SlotMemory: read cached item and due times
ItemScanService->>ItemScanHandler: scan when due
ItemScanHandler->>ItemScanService: return next scan time
ItemScanService->>SlotMemory: store item and next scan time
Suggested reviewers: ryanbarlow97
Merge Risk: 
Low · previous review 15cab
-
Quickly reopening the same container may skip a scan that should run on opening. Reset container memory on open; this is a bounded issue that does not otherwise block merging.
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.
|
@coderabbitai review |
Action performed
|
Reopening the same container before a pulse saw it closed reused its remembered slots, so handlers that skip unchanged items did not see them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@coderabbitai review |
Action not completed
|
Fixed in 86d0880 (container memory reset on open); CodeRabbit confirmed the fix in the thread. Re-review is rate limited.
Why
The shared item scan runs every 2 ticks over every slot of every online player (and open containers), and calls each handler on every item, changed or not. On Main's 11-02 spark profiles
ItemScanService.pulsecost about 14 s per hour (~0.19 ms/tick). Most of that was Cooking's TagManager rewriting every food (6.6 s/h) plus its matching (2.1 s/h) and the fish scan (0.8 s/h). Nearly all of those items had not changed since the last scan.Change
ItemScanHandlergetsscan(player, inventory, slot, stack)returning when the handler needs the slot again:EVERY_SCAN, as now;WHEN_CHANGED, to skip the slot until its item changes;currentTimeMillistime to recheck the unchanged item then (food that ages).scankeeps today's behaviour for matched items (matches → update → every scan). An item that does not match is skipped until it changes, sincematchesonly sees the item, so unchanged slots stop running handlers that can't concern them.ItemScanServiceremembers, per online player and inventory, a copy of each scanned item and each handler's answer:ItemScanService.rescanAll()forgets every skip, for a plugin whose answers depend on its config (call it after a reload).scanin perf: rewrite scanned food only when it would look different Cooking#124) and Magic'sArtifactAttuneScanHandler. Magic keeps the default: itsmatchesreads only the item's own data, and its matched items still run every scan.Testing
ItemScanServiceTest(new cases): unchanged slots skipWHEN_CHANGEDhandlers, a due time brings the slot back, a changed item resets every handler, a handler's own rewrite is seen as a change,EVERY_SCANand the default keep running matched items, non-matching items are skipped,rescanAlland quit forget the memory.mvn -B -o clean verify: 258 tests pass.Dev (TFMCDev01, 11-02, with the Cooking branch): a hot chicken leg in a player's inventory went Hot → Warm at ~11 s and to room temperature at 21.5 s after it was made, the same as the old jars (11.7 s / 21.5 s). It was rewritten 3 times instead of 15, and a raw chicken next to it 0 times instead of 15.
cooking reload→rescanAllleft both items correct. No TLibs warnings or errors in the log.Dev retest with 86d0880 and the Cooking 0.7.2 hotfix build: same food timings and rewrite counts; food left in a chest stayed Hot while it was closed and was aged the moment a bot opened it.
🤖 Generated with Claude Code