Skip to content

Remember the last shared cube per definition - #698

Closed
BonsUnleashed wants to merge 1 commit into
embeddedt:1.20from
BonsUnleashed:bons-furious/modernfix-cube-bake-memo
Closed

BonsUnleashed wants to merge 1 commit into
embeddedt:1.20from
BonsUnleashed:bons-furious/modernfix-cube-bake-memo

Conversation

@BonsUnleashed

@BonsUnleashed BonsUnleashed commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

With compact entity models enabled, repeated bakes of a cube definition construct and hash the same boxed list before finding the cube ModernFix already shares. This remembers the last result on the definition and checks the constructor arguments before building that list again.

The shortcut lives inside the existing constructor wrapper. The rest of bake, including other wrappers around it, still runs. A change to any constructor argument goes through the existing global cache. Floating-point comparisons use the same bit-based equality as boxed floats, and visible faces are copied so an in-place change is noticed. The remembered value is published as one immutable object.

The equivalent optimization is prepared for Bons and Furious 1.0.30. It applies only when ModernFix's existing cube-sharing mixin is enabled.

compileJava passed on Java 21. A direct-source harness matched boxed-key equality in 14,000 checks covering every float field, signed zero, NaNs, infinities, shared cube identity and in-place face-set changes. In a real full-client run, the memo handled about 72% of CubeDefinition calls during each of three resource reloads. Whole reload timings overlapped baseline, and the equipped-player rendering window made no CubeDefinition calls, so no reload or FPS gain is established. This was an activation/timing test, not a visual comparison. Measurements, source and limitations.

@embeddedt

embeddedt commented Oct 4, 2026 via email

Copy link
Copy Markdown
Owner

@BonsUnleashed

Copy link
Copy Markdown
Contributor Author

Understood. The 14,000 checks establish correctness; they don't establish a real-world speedup. I don't have client reload measurements for this PR yet.

I'll add those measurements for these ModernFix changes, with the mod set and test procedure, and use that requirement for future performance PRs too.

@BonsUnleashed

Copy link
Copy Markdown
Contributor Author

I now have results from actual client loads and resource reloads. They show that the memo is used, but do not establish an overall speedup.

With compact entity models explicitly enabled in both baseline and candidate, each of the candidate's three reloads made about 92,000 CubeDefinition calls. About 66,000, or 72%, returned through the new per-definition memo; the remainder used the existing shared-cache path. No repeated bake loop was added to create those hits.

Whole resource reloads had a median of 29.46 seconds for this PR versus 29.35 seconds for baseline, with overlapping ranges. This is one candidate process with three reloads, compared with two baseline processes and six reloads, so it is a small sample. During the 30-second equipped-player rendering window there were no CubeDefinition calls at all. That scene gives no evidence of an FPS benefit.

The measurements and test code include the exact setup, activation counts, timings and limits. The description now distinguishes these results from the earlier correctness checks. On this evidence I can say the shortcut handles real work, but I can't claim the real-world performance improvement you asked for.

@embeddedt

Copy link
Copy Markdown
Owner

So... this adds 68 lines of complexity and fragility (memoization is easy to get wrong), for no real-world improvement. Sorry but this is not mergeable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants