Skip to content

perf: demote audited movement and menu packets to narrower execution lanes - #553

Draft
carabistouflette wants to merge 1 commit into
Steel-Foundation:masterfrom
carabistouflette:pr6-packet-lane-audit
Draft

perf: demote audited movement and menu packets to narrower execution lanes#553
carabistouflette wants to merge 1 commit into
Steel-Foundation:masterfrom
carabistouflette:pr6-packet-lane-audit

Conversation

@carabistouflette

Copy link
Copy Markdown
Contributor

Type of change

  • Block implementation
  • Item implementation
  • Command implementation
  • Entity implementation
  • Bug fix
  • New feature
  • Breaking change
  • Refactor / code cleanup
  • Performance improvement

Description

Concurrency audit of the scheduled packet lanes (see per-handler notes in ScheduledPlayPacketKind::execution):

  • MovePlayer / MoveVehicle → PlayerLocal (was Serialized). The handlers mutate only per-player movement and teleport state behind SyncMutex and route through chunk-map ticket APIs with fine-grained internal locks; collision lookups are read-only. This takes the hottest packet path off the global serialized lane.
  • ContainerButtonClick / ContainerSlotStateChanged → Serialized (was Exclusive). Menu transactions serialize on the menu lock exactly like ContainerClick, so the full global barrier was unnecessarily wide.
  • Attack, Interact, CustomPayload, PlayerAction::ReleaseUseItem remain Exclusive: combat spans independently locked source and target state; custom payloads have no constrained resource contract.

The ScheduledPacketExecution doc comment documents the per-lane safety invariants.

How this was tested

  • Classification tests updated: packet_execution_classification_separates_local_and_serialized_work now asserts MovePlayer = PlayerLocal; new menu_transactions_share_the_serialized_lane asserts Click/ButtonClick/SlotStateChanged = Serialized; cross_player_handlers_remain_global_barriers (renamed) asserts Attack/Interact/ReleaseUseItem = Exclusive.
  • cargo test -p steel-core --lib (2423 tests) pass; cargo clippy -r --all-targets clean.

Screenshots / logs

N/A.

Checklist

  • Code builds w/o errors or warnings
  • Self-reviewed the diff
  • Docs updated (if applicable) — N/A
  • No leftover debug code / comments

Additional notes

  • Contingency from the audit plan: if profiling surfaces unexpected contention on chunk-map tickets under MovePlayer concurrency, MovePlayer can fall back to Serialized while keeping the menu demotions.
  • Env: Linux, Rust nightly, Minecraft protocol 776 (0.15.2+mc26.2).

…lanes

Concurrency audit of the scheduled packet lanes:
- MovePlayer and MoveVehicle handlers mutate only per-player movement and
  teleport state behind SyncMutex and route through chunk-map ticket APIs
  with fine-grained internal locks; they are demoted from Serialized to
  PlayerLocal, taking the hottest packet path off the global lane
- ContainerButtonClick and ContainerSlotStateChanged join ContainerClick on
  the Serialized lane: menu transactions serialize on the menu lock, so the
  previous Exclusive barrier was unnecessarily wide
- Attack, Interact, CustomPayload, and ReleaseUseItem remain Exclusive
  (combat spans independently locked source and target state; custom
  payloads have no constrained resource contract)

Classification tests updated accordingly, with a new menu-lane test and an
Interact barrier assertion.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants