Skip to content

perf: look up each node's guild and each guild's limits once per pass - #33

Draft
XxFran10xX wants to merge 1 commit into
mainfrom
perf/node-pass-lookups
Draft

XxFran10xX wants to merge 1 commit into
mainfrom
perf/node-pass-lookups

Conversation

@XxFran10xX

Copy link
Copy Markdown
Contributor

What this changes

Performance only. Players see no difference.

The particle pass (NodeManager.particleCycle, every 5 ticks) and the upkeep total that SimpleFactions reads asked for the same things several times per node. Each is now asked once per pass or per call.

Where Before After
Particle pass, per claimed node The node's guild looked up 3 times: once to see if the node is claimable, twice more inside Node.check Looked up once and handed to Node.check(Guild)
Count at the start of the pass Guild looked up twice per node (hasGuild, then getGuild) Once
Node.canHold in the pass Worked out for every node: leader's character lookup, member count, node count, capacity Worked out once per guild per pass, because it depends only on the guild
Node.canHold, leader lookup Always done first Done only for a guild that is short of members or over capacity
getTotalUpkeep, getNodeAmount Guild looked up twice per node Once

Why

A spark profile of Main (60 players, one hour; 72,000 ms per hour is 1 ms per tick) showed:

  • The particle pass: 6.5 s/h.
    • Node.check 4.1 s: canHold 3.1 s (leader lookup through Members.player into TLibs Characters.info 1.7 s, capacity 1.0 s), and isClaimable → resolveGuild 0.6 s.
    • beginCountPass 1.7 s, of which hasGuild → resolveGuild → FactionManager.getGuildByString 0.85 s.
    • isClaimable → resolveGuild called straight from the pass: 0.6 s.
  • NodeManager.getTotalUpkeep (SimpleFactions' Ledger.liveNodeUpkeep): 2.7 s/h, mostly hasGuild / getGuild → resolveGuild. getNodeAmount has the same pattern, about 1.1 s/h.

The profile is from before #31, so its capacity line was getNodeCapacity; the same check now uses getHeldNodeCapacity.

Why the result is the same

  • Nothing is remembered between passes. Both pass memos are created in beginCountPass and dropped in endCountPass, so a deleted or renamed guild and a node that changes owner are seen on the next pass exactly as before.
  • The lookups that were merged ran back to back with nothing in between that could change the answer.
  • canHold reads only the guild: held realm, leader, member count, node count, capacity. Within one pass these change only when a node is let go, which lowers the count and resyncs realms. released clears the memo at that point, so the next node of that guild is judged afresh, as before.
  • The canHold reorder returns the same value: held, or within member and capacity limits, or led by an online admin. Only the order of the last two changed, and both are plain reads.
  • While SimpleFactions is not ready the pass still does nothing for a node, as before.

Left as it is

  • A pass still looks each claimed node's guild up twice: once for the count and once for the check. Reusing the first for the second would need its own "a node was let go" invalidation, for about 0.4 s/h that the SimpleFactions-side change to getGuildByString removes anyway.
  • getTotalUpkeep(g) still looks up the guild of every active node, once each. Skipping the other guilds' nodes without a lookup would rely on how SimpleFactions matches ids.
  • For the few nodes that do belong to the guild, getDailyUpkeep still resolves the guild and its capacity per node.
  • The 30 s realm check and the one-minute production loop use the same hasGuild then getGuild pattern, but they were not in the profile.

How it was tested

New and adjusted unit tests:

  • A pass over real nodes makes 2 guild lookups per claimed node and works canHold out once for a guild with two nodes.
  • Within a pass canHold is worked out once per guild, again after a node is let go, and nothing is kept once the pass ends.
  • A guild within its limits keeps its nodes without its leader being looked up; a guild short of members does have it looked up.
  • While SimpleFactions is not ready a pass neither checks nodes nor shows the claimable particle.
  • Node.check makes one guild lookup.
  • Existing tests that stubbed hasGuild() on mock nodes now stub getGuild(), which the counting code reads.

mvn -B clean verify on Windows with JDK 21: 258 tests, 250 pass, 8 fail. The same 8 fail on origin/main on this PC and are not touched by this change. They write files and fail with AccessDeniedException in DurableFiles.write:

  • DatabaseTest: jsonAccessorsAndSavePreserveTypedDefaultsAndExistingValues, snapshotSavesBeforePruningAndPreservesUnknownFiles
  • NodePaymentsTest: conflictingTargetStateAndInvalidJournalNeverPost, conflictsAndChangedOwnersRemainBlockedForStaff, crashAfterPostingReappliesTheSavedTargetAndNeverIncrementsAgain, failedCapacitySaveIsReplayedWithSamePayment, refusalLeavesStateWhileUnavailableBlocksNewIntentAndRecoversSameId, successfulChangesPersistExactLevelCapacityAndRefund

Because of them the local JaCoCo gate reports 30 missed lines, all in NodePayments.java, Database.java and DurableFiles.java. Node.java and NodeManager.java have no missed lines. The Linux CI run is the real check of the 100% gate.

Not measured on a live server. The saving is in lookup counts; a new spark profile after the next release will show the time.

🤖 Generated with Claude Code

The particle pass runs four times a second and asked SimpleFactions for the
same things several times per node. A spark profile of Main (60 players, one
hour) put it at 6.5 s of the server thread, with the upkeep total SimpleFactions
reads at another 2.7 s.

- The pass looks a node's guild up once and hands it to the check, instead of
  once to see if the node is claimable and twice more inside the check. The
  count at the start of the pass looks it up once instead of twice.
- Whether a guild can hold its nodes depends only on the guild, so a pass works
  it out once per guild instead of once per node. Letting a node go changes the
  count and resyncs realms, so the answers are forgotten then.
- A guild within its member and capacity limits keeps its nodes without its
  leader being looked up. The lookup reads the leader's character and only
  matters for a guild that would otherwise lose the node.
- The upkeep total and the node count look each node's guild up once instead
  of twice.

Nothing is kept between passes, so a deleted or renamed guild and a node that
changes owner are seen exactly as before.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Nov 11, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • 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

@github-actions github-actions Bot added the size:M 30-99 effective changed lines (tests excluded in mixed PRs). label Nov 11, 2026
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