Skip to content

perf: skip debug log building and re-placing standing trains every tick - #85

Merged
XxFran10xX merged 1 commit into
mainfrom
perf/idle-train-and-logs
Oct 29, 2026
Merged

XxFran10xX merged 1 commit into
mainfrom
perf/idle-train-and-logs

Conversation

@XxFran10xX

Copy link
Copy Markdown
Contributor

Why

VehicleManager's fast tick runs every loaded vehicle on every tick. Two things did work there that changed nothing:

  • Debug logs built their lines while logging was off. RecorderLog.pose built a "pose:"+uuid key and did a map lookup for each car on each tick. Up to 4 times a second it also did about 9 String.format calls plus train(v). PersistenceLog.applyPose built a long line from xyz() of 3 locations (each one calling loc.getChunk() twice) and vehicle(). Both lines were then thrown away. placeCars, particleOffset, playback and the rest followed the same pattern.
  • Standing trains were placed again on every tick. placeLoadedCars → applyPlacements → applyPose teleported each car, zeroed its velocity and ran the quaternion turn every tick, even when nothing had moved.

Changes

  • RecorderLog / PersistenceLog: a private off() (!enabled || logFile == null). pose, sample, playback, interact, arm, junction, placeCars, applyPose, particleOffset, spawn*, save*, loadJson, tryLink, mount, remount and dismount now return before building anything. Two calls keep their side effects: spawnManagerStart still advances the spawn cycle, and unload still clears the throttle maps. The formatters xyz/vehicle/train are unchanged.

  • TrainHandler.applyPose: the teleport and setVelocity(0) are skipped only when all of these hold:

    • the entity is the same one this car last placed;
    • it is within 1e-6 blocks of the pose;
    • its velocity is exactly zero;
    • it is not a Mob.

    setGravity(false), deck placement, the body turn, the logs and the track crumbs still run every tick. posedEntity is cleared in applyConsist and unbind, so the first pose after a load or a bind is always applied in full.

  • BoneRotator.rotateToTarget: the old body moved into a private turnToTarget. The wrapper remembers the last call that left the rotation bit-for-bit unchanged. When the same call comes again (same arguments, same rotation, and driveYaw already at the target yaw), it returns the remembered result without doing the turn.

Why behaviour is identical

  • Logs: the code after each guard is unchanged. A golden test checks exact line text for POSE, SAMPLE, PLAY, INTERACT, ARM, JUNCTION, SPAWN_MANAGER_START, PLACE_CARS, APPLY_POSE, SMOKE_OFFSET, SPAWN_LOAD and UNLOAD, and it passes against both origin/main and this branch.

    • With logging on from startup, nothing differs.
    • If logging is switched on later by a reload, the per-vehicle n= counters start from 0 instead of counting the ticks when it was off.
    • With logging off, the logs no longer call Location.getChunk(), which could read a chunk that is not loaded. See Notes.
  • Teleport skip: the target comes from (spline, s), not from the car's current position. A standing car's target is therefore bit-identical every tick, and the most a car can differ from its target is 1e-6 blocks; there is no drift that builds up over time. The car is still put back when:

    • something moved it (explosion, piston, chunk reload): the position check fails;
    • something pushed it (knockback, explosion velocity): velocity is no longer zero. This catches it before the entity's own tick moves it.

    teleport(next) keeps the entity's own yaw and pitch, so teleporting a still car that is already at next changes nothing on an ArmorStand base, the default. Mob bases are always teleported, because there the teleport also brings the body back in line with the head.

  • Rotation skip: turnToTarget depends only on its arguments, the animator's rotation and driveYaw. If one call left the rotation unchanged, the same call from the same state leaves it unchanged again. A test checks that a fresh rotator, run in full from the settled rotation, gives the same result.

    • On a standing train the old code moved the roll by one float step each tick for about 10 to 300 ticks before it reached an exact fixed point. The skip only starts once that point is reached.
    • The same applies to bogie rotators and to every other caller.

Testing

  • New tests:
    • TrainStandingPoseTest: standing cars are placed once and then left alone; the first placement applies even when a car is already at its pose; a pushed car snaps back, even after a 1e-4 push; a car set moving is stopped; a respawned entity, an applyConsist and an unbind+bind each trigger a full placement; Mob cars are always teleported; the deck is placed every tick; a moving train is teleported every tick.
    • BoneRotatorSettleTest: settles across yaws including ±180; the skipped turn equals the full turn; an outside turn is put back; a re-read driveYaw is restored; other turns are not skipped.
    • RecorderLogQuietTest / PersistenceLogQuietTest: with the log off, verifyNoInteractions on the vehicle and locations; with the log on, the golden lines above.
  • The new off-path and pose tests fail on origin/main, as expected.
  • Local Windows mvn -B -o clean verify: 1815 tests, 0 failures, 9 skipped. The coverage gate misses the same 10 lines as the origin/main baseline (VehicleRepository, TrackStore and template loaders, all covered by tests that are skipped on Windows). Linux CI is authoritative.

Notes

  • With logging off, the log calls no longer touch Location.getChunk(). On origin/main they did this on every tick, including for the pose target, which can lie in the next chunk ahead of a moving train. No gameplay effect is expected, but trains running at the edge of loaded chunks are worth watching on Dev.
  • The larger change that would skip parked vehicles entirely is not in this PR.
  • Dev testing follows: standing and moving trains, multi-car consists, a tape route with dwell, a knockback or explosion against a standing car, deck riders, and a chunk reload.

🤖 Generated with Claude Code

… tick

The recorder and persistence logs now return before building a line,
key or map lookup when their log is off. Lines written with the log on
are unchanged.

A train standing on its track is posed every tick. A car that is
already still at its pose (same entity as last placed, within a
millionth of a block, zero velocity, not a Mob) is no longer teleported
and stopped again. Gravity, deck placement and the body turn still run
every tick. A moved or pushed car fails the check and snaps back as
before. The first pose after loading or binding is always applied.

BoneRotator.rotateToTarget skips a call that repeats the last call that
left the bone unchanged, from that same rotation. The turn depends only
on its arguments and the rotation, so the skipped call is a no-op.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: fd59b689-a815-4cae-bd51-608dd7de1ad0
Commits

Reviewing files that changed from the base of the PR and between 2f6d268 and 5cc374e.

Files selected for processing: 8
  • src/main/java/net/tfminecraft/vehicleframework/bones/BoneRotator.java
  • src/main/java/net/tfminecraft/vehicleframework/database/PersistenceLog.java
  • src/main/java/net/tfminecraft/vehicleframework/tracks/RecorderLog.java
  • src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainHandler.java
  • src/test/java/net/tfminecraft/vehicleframework/bones/BoneRotatorSettleTest.java
  • src/test/java/net/tfminecraft/vehicleframework/database/PersistenceLogQuietTest.java
  • src/test/java/net/tfminecraft/vehicleframework/tracks/RecorderLogQuietTest.java
  • src/test/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainStandingPoseTest.java

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Summary

Summary by CodeRabbit

  • Performance

    • Repeated bone rotations are skipped when the target and relevant movement settings are unchanged, while changes in rotation or ongoing movement still trigger an update.
    • Stationary train cars no longer repeat placement and velocity-reset actions on every update. Cars are still repositioned when displaced, moving, or replaced.
    • When logging is disabled or unavailable, log messages are no longer built, reducing unnecessary work.
  • Bug Fixes

    • Train placement continues to handle moving cars, newly placed cars and mob cars appropriately.

The change adds settled-call caching to bone rotation, early returns to persistence and recorder logging when logging is off, and conditional placement for standing train entities. New tests cover these behaviours and check that enabled logging retains its expected output.

Changes: Bone rotation settling

Layer / File(s) Summary
Settled turn cache and validation
src/main/java/net/tfminecraft/vehicleframework/bones/BoneRotator.java, src/test/java/net/tfminecraft/vehicleframework/bones/BoneRotatorSettleTest.java
rotateToTarget returns a stored result when the call inputs and rotation state match a settled call. Tests cover matching calls, changed inputs, external rotation and drive yaw.

Disabled logging

Layer / File(s) Summary
Persistence log guards and tests
src/main/java/net/tfminecraft/vehicleframework/database/PersistenceLog.java, src/test/java/net/tfminecraft/vehicleframework/database/PersistenceLogQuietTest.java
Persistence logging methods return early when logging is disabled or the log file is unset. Tests cover disabled logging, spawn-cycle updates and selected enabled log output.
Recorder log guards and tests
src/main/java/net/tfminecraft/vehicleframework/tracks/RecorderLog.java, src/test/java/net/tfminecraft/vehicleframework/tracks/RecorderLogQuietTest.java
Recorder logging methods return early when logging is disabled or the log file is unset. Tests cover disabled and enabled logging.

Standing train placement

Layer / File(s) Summary
Pose tracking and conditional placement
src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainHandler.java, src/test/java/net/tfminecraft/vehicleframework/vehicles/handlers/TrainStandingPoseTest.java
applyPose skips teleporting and clearing velocity when the same entity remains at the target pose and is still. Consist changes and unbinding clear the cached entity. Tests cover movement, displacement, entity changes and mob cars.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Refactor

Possibly related PRs

Suggested reviewers: ryanbarlow97

Merge Risk: Minimal · up to 5cc37

The reviewed changes are mergeable after normal checks; no actionable risk remains identified.

- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the size:L 100-499 effective changed lines (tests excluded in mixed PRs). label Oct 29, 2026
@XxFran10xX

Copy link
Copy Markdown
Contributor Author

Dev test (TFMCDev01, 10-29 13:26 UTC, this PR's jar; ~/perf-1029-test/train.js on tf): PASS

Two parked consists near 5945,239,5229 (3 train bases tracked from a bot's view):

vehicleframework-2.17.1 this PR
largest move of a parked base in 5 s 0.000000 0.000000
position packets for the 3 bases in 5 s 300 entity_velocity + 6 rel_entity_move 6 rel_entity_move
base nudged 0.4 blocks by /tp: offset 0.15 s later 0.000 0.000
VF errors 0 0

So standing cars stay put, no longer send a zero-velocity packet every tick, and still snap back at once after an outside move.

@XxFran10xX
XxFran10xX marked this pull request as ready for review October 29, 2026 13:38
@XxFran10xX

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@XxFran10xX
XxFran10xX merged commit 205f628 into main Oct 29, 2026
4 checks passed
@XxFran10xX
XxFran10xX deleted the perf/idle-train-and-logs branch October 29, 2026 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 effective changed lines (tests excluded in mixed PRs).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant