Remove Redundant Caching on Surface Rule Conditions #585
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
This PR deals with some issues with Vanilla and (more) ineffective caching on the worldgen side, within surface rules.
The Issue With Vanilla Behavior
Because of how Vanilla codecs create an object per non-interned JSON object, interactions
LazyYCondition(which should beLazyXYZCondition, because it's per-position), cause the caching implemented onLazyYConditionto be actively detrimental, as the cache is unused most of the time.For the cache to be used, it must be either interned (not currently done) or an equivalent must happen, such as with
TemperatureHelperCondition, where a single instance is stored in theSurfaceRules#Contextand accessed from there by theConditionSource. Without that, theLazyYCondition-inherting condition is evaluated once, before the cache is never accessed again due to the change in the position; which means all of the caching behavior is and time and memory spent on creating a cache that is then destroyed when the next position is moved to.Parameterized conditions are never stored; this includes most of the commonly used position-specific conditions, including
WaterConditionandStoneDepthCondition, alongside the rare but highly-appliedVerticalGradientCondition.Why This PR Doesn't Intern The Values
I believe the value of interning conditions would depend upon which conditions. Most* of the time in Vanilla or Vanilla-adjacent worldgen, there's not a lot of highly duplicate equivalent conditions. For some (water depth, stone depth) there are relatively few max total checks per object, and throughout the entirety of the Vanilla rule set, and you're not going to be hitting all of them at once (nor do all have the same parameters)
Even in old versions of my worldgen mod (Natural Philosophy), which use the Lithostitched surface rule injection system and thus duplicate many checks due to the split nature of their files, I don't think there's more than about sixty equivalent checks of any given type in total throughout the entirety of files, and any given block would at absolute max encounter no more than roughly forty of the same check at once - and not all of those would intern to the same values. That's if every file is checked against a single block before it happens to hit the right file.
Even this relatively large number of conditions is slated to be changed due to outsized performance impact, and I have a relatively easy method of doing so, which will in sum total produce better performance than implementing the interning and intended caching behavior of the conditions.
For things like
VerticalGradientConditionSource, I believe there is even less reason to intern the condition and allow caching. The only time that condition exists in Vanilla is bedrock and deepslate. Interning those would not make them one object - two different heights and different blocks means different variables - and so you'd be caching the result for something simply not applied anywhere else, while the checks are run on basically every block.Possible Routes to Interning Values With Caching
To determine which conditions would be better left interned, I would want to run a deeper comparative performance analysis on a simulated surface system that can accurately mimic Vanilla generation without having randomness, with both "Vanilla case", and "Worst likely worldgen mod case" tests to figure out which is actually comparatively faster for most users. The specific methods that Terrablender does their surface rule addition with would also I think be of interest to analyze, as I haven't looked into their code, and I don't know if they have repeat checks with how mods commonly add rules with that.
Performance Benefits
From testing with the code in a standalone mod as well as within ModernFix, I expect a roughly 10% boost to the speed of surface rule generation for a complex (windswept) Vanilla terrain; this will not map directly to worldgen performance boosts, I expect it to produce a roughly 5% bonus to worldgen performance due to it being sequentially done on-worldgen-thread with biome decor and structures.
Statistics are included below; please do ask if you want to know more about my methodology.

Mixin Category Enables
This PR enables the
Worldgen Performancemixin category; it also has been tested to ensure compatibility.The following mods have been tested and do not seem to cause issues: