fix: PDC ownership tags + orphan entity audit/cleanup for display leak - #130
Open
Arbousier1 wants to merge 17 commits into
Open
fix: PDC ownership tags + orphan entity audit/cleanup for display leak#130Arbousier1 wants to merge 17 commits into
Arbousier1 wants to merge 17 commits into
Conversation
Adds a Map-based placeholder overload to LocalizedMessages (behaviourally identical delegation to the TagResolver variant) so hot rendering paths can be benchmarked through a stable placeholder representation, plus a new JMH benchmark covering the Spark-sampled call sites (bot names, seat status, empty seat, centre line) and an i18n gate profile with time/alloc metrics.
test_gate.py assumes benchmarks[0] is the infra entry; inserting the i18n entries at the head broke validate_profile_config for the infra profile (must_pass not subset of primary ids from the stubbed specs). Keep array order stable by appending new entries at the tail.
std::regex construction compiles the pattern on every call; the StringToHandtiles parse runs on each JNI evaluateFan/evaluateTing, and std::__detail::_Compiler frames dominated native sampling. A function-local static (C++11 thread-safe init) compiles once.
perf: add i18n benchmark profile for parametrised message rendering
perf(native): compile handtiles legality regex once as static
Spark sampling (j9YE2XtMP3) shows per-tick viewer rendering spends its time in MiniMessage deserialization under LocalizedMessages.plain with placeholder TagResolvers: - Cache Map-based render/plain by (locale, key, sorted placeholder entries) via Caffeine (max 512): hot paths publicSeatStatus, publicCenterText and botDisplayName now hit a single deserialize per stable placeholder set instead of per tick. - Add formatNumber + Map overloads to MessageService facade so hot callers can use the cached path without constructing TagResolvers. - Reuse one PlayerActionSnapshotFactory per session (was 3-4 news per bot tick) and replace hasAction stream().anyMatch with a plain loop. Behaviour is unchanged: placeholder maps are resolved by tag name, so iteration order does not affect output; bundles are immutable after load.
BotActionSchedulerTest and SichuanBotPreparationSchedulerTest mock MahjongTableSession, which now exposes actionSnapshotFactory(). Stub it to return a real PlayerActionSnapshotFactory so the shared per-session factory path is exercised by the scheduler tests.
perf(i18n): cache parametrised renders and reuse action snapshot factory
…spawn on re-arrangement Spark profile (spark.lucko.me/8yCLAsdfdV) showed applyRenderPrecompute as the largest mahjong hotspot (27%): every hand re-arrangement (deal/draw/discard) shifted all tile positions, which changed every tile fingerprint (point was part of the content fingerprint) and forced a full remove+respawn of 26+ regions per seat while reconcile (teleport) was almost never reached. - handPrivateTileFingerprint/handPublicTileFingerprint no longer mix tile positions into the content fingerprint; new privateHandLayoutFingerprint / publicHandLayoutFingerprint track the layout separately. - TableRegionDisplayCoordinator keeps per-region appliedLayoutFingerprints; a layout-only change now enters the render path and is satisfied by reconcile (entity teleport) instead of respawn, with respawn retained as a safe fallback. - cache rendered bot display names (i18n) per player+locale, invalidated on participant changes, cutting per-tick displayName rendering.
…mplete The first A/B run showed a 39.8% regression on region.apply-deferred because the per-seat layout fingerprints enumerated every tile point (320-capacity builder, ~150 FNV mixes each) on every apply, even on the short-circuit path. Tile coordinates are fully determined by (seat, hand size, tile index, selected indices) — all already present in the per-tile content fingerprints once hand size is added to the public tile fingerprint — so the layout fingerprint only needs to carry the hand structure: - handPublicTileFingerprint now includes hand size, making public hand content fingerprints layout-complete again; - privateHandLayoutFingerprint/publicHandLayoutFingerprint shrink to a few structural FNV fields (computed once per seat per apply, never enumerating coordinates); - both short-circuit paths drop the per-region layout lookup and go back to a pure content-fingerprint comparison; - the reconcile-vs-respawn decision on the update path keeps the layout gate: structure unchanged -> reconcile (teleport in place), structure changed (hand grew/shrunk) -> respawn. Net effect: the every-tick apply fixed cost returns to baseline while the real-game win (layout-only changes no longer poison every tile fingerprint) is preserved.
The A/B rerun (dedicated runner, 8 iterations) shrank the apply-deferred regression from -39.8% to -6.1% and flipped complete-fingerprints to +3.04% PASS_OPTIMIZED, but the remaining regression tracks the extra hand-size field added to handPublicTileFingerprint (one extra FNV mix per tile per apply). Tile index and hand size are packed into a single injective field (tileIndex * 64 + handSize, both < 64 in every legal game state), so the public tile fingerprint keeps exactly the pre-change field count and the every-tick short-circuit fixed cost returns to baseline while the layout-complete content semantics are preserved.
The A/B rerun still showed a -7.25% apply-deferred regression. The benchmark drives the planning coordinator with a zero budget, so every apply only pays the enqueue cost: fingerprint computation plus the per-seat layout fingerprint precomputation, which is never consumed on the short-circuit path. Since tile coordinates are fully determined by (seat, hand size, tile index, selected indices) — all covered by the content fingerprints — the layout structure fingerprint reduces to the hand size itself. Use the already-computed handSize value as the fingerprint (long) instead of calling the fingerprint service, removing the last per-apply computation added by the layout/content split. The reconcile-vs-respawn gate keeps comparing the applied hand size with the current one. The now-unused privateHandLayoutFingerprint/publicHandLayoutFingerprint methods are removed.
…fingerprint Shaves the last per-apply arithmetic (a multiply) off the short-circuit path; field count and injectivity are unchanged.
Region fingerprints are compared per region key (hand-private:WIND:index), so the type prefix field is redundant and costs one FNV mix per tile per apply. Removing it from the hand tile fingerprints restores the short-circuit fixed cost to below baseline on the apply benchmarks.
perf: layout/content fingerprint split + bot display-name cache (PR-D)
Tag every spawned region entity with table/session/role/slot/generation in its persistent data container, add /mahjong cleanup (orphan-only) and debug entities listing, and scan for orphans at plugin startup so leaked displays can be attributed to a table and removed without ever touching unrelated Interaction/ItemDisplay entities.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DisplayEntities新增 5 个 PDC tag（table_id/session_id/role/slot/generation），TableRegionDisplayCoordinator两处 respawn 路径（hand-private / hand-public）对每个新生成的实体写归属，generation 为协调器内单调递增代际计数ManagedEntityAudit，只统计/清理确认无归属（无 table_id 或归属 table 不存在）的受管实体；CleanupSubcommand提供/mahjong cleanup清理命令，DebugSubcommand新增 entities 分支列出全部受管实体（位置/类型/归属/世界）Why
线上卡顿根因：JVM 堆顶满 → 频繁老年代 GC（5.33GB 存活对象）。heap dump 诊断前需要实体级归属信息，现有
mahjong:managed标记无法区分归属/代际，无法判断泄漏实体属于哪张桌、哪一轮生成。Test plan
./gradlew compileJava compileTestJava -PmahjongJavaToolchain=25 -PmahjongJavaTarget=21./gradlew spotlessJavaCheck/mahjong debug entities列出实体与归属；/mahjong cleanup清理孤儿