Skip to content

perf: skip unchanged estate receipt writes and extra directory flushes - #47

Merged
XxFran10xX merged 4 commits into
mainfrom
perf/estate-skip-unchanged-writes
Oct 23, 2026
Merged

XxFran10xX merged 4 commits into
mainfrom
perf/estate-skip-unchanged-writes

Conversation

@XxFran10xX

Copy link
Copy Markdown
Contributor

Summary

Stops the hourly table-estate pass from rewriting estate receipts that haven't changed.

TableEstate.reconcile runs every 72000 ticks (and on estate/revive events) and settles every decided estate again. For a retained estate (no character recipient) settle rewrote the receipt with DurableFiles.write every time, even though the content was identical. Each write made a temp file, forced it to disk, moved it into place, and then forced every folder up to the filesystem root to disk.

On Main, all 13 receipts in plugins/Games/estates/tables get new timestamps every hour. In a spark profile (2026-10-22 21:13 UTC) this cost 0.7 s on the server thread in one go (TableEstate.reconcile → DurableFiles.write/sync). Decided estates only accumulate, so the hitch would keep growing, like Magic's 20–30 s freezes fixed in Magic 0.12.0.

Changes:

  • DurableFiles.writeIfChanged publishes only when the file doesn't already hold exactly these bytes. When the content is identical it still flushes the parent folder entry, so a receipt moved into place by an earlier write whose folder flush failed is still made durable. The retained-estate path uses it.
  • DurableFiles.write flushes the parent folder, plus only the folders this write created. It no longer flushes every ancestor up to /. The temp-file + force + atomic-move guarantee is unchanged.

Outcomes reported to Estates are unchanged. Estates that are already finished are still settled each hour: that pass is how "Completed table reappeared at the testator" and history conflicts are detected. It just no longer writes anything.

Testing

  • TableEstateTest: new repeatedRetainedSettlementKeepsItsUnchangedReceiptFile settles twice and checks the outcome is equal and the receipt's bytes and last-modified time are unchanged.
  • DurableFilesTest (new): creating missing folders, no .pending- leftovers, writeIfChanged skips only identical bytes, delete.
  • TableEstateTest + DurableFilesTest: 26/26 pass locally.
  • The full local run has 12 failures, all in TableManagerPersistenceTest/TableManagerLifecycleTest. They're identical on main: Windows refuses to force a directory to disk. Linux CI is the reference.
  • TFMCDev01: Dev's 7 receipts were rewritten at 21:34 and 22:34 on the old jar. With this build, Dev restarted at 23:24 and 23:30 and the receipts kept their 22:34 timestamps. No Games errors in the log.

Part of the 2026-10-22 ping/lag investigation, together with VehicleFramework and RPCharacters PRs.

🤖 Generated with Claude Code

XxFran10xX and others added 2 commits October 22, 2026 23:56
The hourly table-estate reconcile re-settles every decided estate. A
retained (non-character) estate rewrote its unchanged receipt each time,
and every DurableFiles write flushed each ancestor directory up to the
filesystem root. On Main that was a 0.7 s main-thread pause per hour,
growing as decided estates accumulate.

- DurableFiles.write flushes the target's parent, plus the parents of
  directories this write created, instead of every ancestor.
- DurableFiles.writeIfChanged skips the write when the file already holds
  the same bytes; the retained-receipt path uses it. Outcomes are unchanged.

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

coderabbitai Bot commented Oct 23, 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: 8ba4396e-50dd-4aea-b3ee-eb6dbc7a2ed8
📥 Commits

Reviewing files that changed from the base of the PR and between ccc44df and 1d8d150.

📒 Files selected for processing: 6
  • src/main/java/net/tfminecraft/games/Games.java
  • src/main/java/net/tfminecraft/games/ownership/TableEstate.java
  • src/main/java/net/tfminecraft/games/persistence/DurableFiles.java
  • src/main/java/net/tfminecraft/games/table/TableManager.java
  • src/test/java/net/tfminecraft/games/ownership/TableEstateTest.java
  • src/test/java/net/tfminecraft/games/persistence/DurableFilesTest.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

  • Bug Fixes
    • Improved the reliability of saved game data when folders are created or files are updated or deleted.
    • Repeatedly settling the same estate decision no longer rewrites an unchanged receipt, helping preserve its existing contents and modification time.
    • Failed folder flushes can be retried, including when a subsequent save contains no changes.

DurableFiles now tracks directory durability, supports writes that skip identical content, and flushes a deleted file’s parent. Directory-creation callers clear tracked state for newly created directories. Retained-property receipts use conditional writes.

Changes: Durable file persistence

Layer / File(s) Summary
Tracked flushing, conditional writes, and validation
src/main/java/net/tfminecraft/games/persistence/DurableFiles.java, src/test/java/net/tfminecraft/games/persistence/DurableFilesTest.java
DurableFiles tracks directories whose ancestor chains have been flushed. It adds writeIfChanged and forget, and syncs a deleted file’s parent. Tests cover directory creation and recreation, flush retries, unchanged content, and deletion.
Directory creation and receipt persistence
src/main/java/net/tfminecraft/games/Games.java, src/main/java/net/tfminecraft/games/table/TableManager.java, src/main/java/net/tfminecraft/games/ownership/TableEstate.java, src/test/java/net/tfminecraft/games/ownership/TableEstateTest.java
Directory-creation callers forget tracked state when they create directories. TableEstate persists retained-property receipts with writeIfChanged. A regression test checks that repeating settlement preserves receipt contents and modification time.

Priority: ➖ Normal

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

Change: Refactor

Sequence Diagram(s)

sequenceDiagram
  participant TableEstate
  participant DurableFiles
  participant FileSystem
  participant Syncer
  TableEstate->>DurableFiles: writeIfChanged(target, receipt)
  DurableFiles->>FileSystem: Check existing regular-file contents
  alt Contents are identical
    DurableFiles->>Syncer: Flush parent entry
    DurableFiles-->>TableEstate: Return false
  else Contents differ
    DurableFiles->>FileSystem: Write through atomic replacement
    DurableFiles->>Syncer: Flush parent entry and untracked ancestors
    DurableFiles-->>TableEstate: Return true
  end
Loading

Suggested reviewers: ryanbarlow97

Merge Risk: ⚪ Minimal · up to 1d8d1

No actionable merge-blocking risk is established for the changed receipt-write and directory-flush behavior. The change is mergeable after normal checks.

- [ ] 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:M 30-99 effective changed lines (tests excluded in mixed PRs). label Oct 23, 2026
XxFran10xX and others added 2 commits October 23, 2026 02:21
Sol review: a folder created by mkdirs elsewhere, or by a write whose ancestor
flush failed, still needs its ancestors flushed. Track directories (by file key)
whose whole chain has been flushed; flush the chain on the first write there
and again after any failure, and only the parent afterwards.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sol review round 2: file keys can be reused after a folder is deleted, so key the
flushed-folder cache by path and forget a folder (and everything under it) when
a durable write or Games' own mkdirs creates it, instead of trusting the key.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@XxFran10xX
XxFran10xX marked this pull request as ready for review October 23, 2026 00:27
@XxFran10xX

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 23, 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 2b0887d into main Oct 23, 2026
4 checks passed
@XxFran10xX
XxFran10xX deleted the perf/estate-skip-unchanged-writes branch October 23, 2026 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 effective changed lines (tests excluded in mixed PRs).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant