Skip to content

Releasing blending layer data - #63

Open
aqpanaciy wants to merge 1 commit into
developfrom
aqpanaciy-blend
Open

aqpanaciy wants to merge 1 commit into
developfrom
aqpanaciy-blend

Conversation

@aqpanaciy

Copy link
Copy Markdown
Collaborator

The blending layer is used only for calculating terraforming barriers. There is no need to store it. Reduces server memory consumption by 260 MB in the current zone configuration.

- the blending layer is used only for calculating terraforming barriers. There is no need to store it. Reduces server memory consumption by 260 MB in the current zone configuration.
@Sellafield

Copy link
Copy Markdown
Contributor

Honestly, I'm unsure about that. From what I know, blend layer is used to store terraforming data and 'degrading' island to it's original state. We have to be extremely careful touching that. and if in return we only get 260MB of RAM it might not worth it.

Please evaluate potential impact.

@clouths clouths left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes looks good.

@Sellafield I've done an impact analysis. The restore-to-original-state path doesn't touch the blend layer:

  • ZoneRestoreOriginalGamma restores from TerraformableAltitude.OriginalAltitude which is a separate layer that stays retained which this PR does not touch. The PBSHelper reads OriginalAltitude too.
  • The blend layer has exactly one read site in the codebase TerraformtableAltitude constructor, where it's mixed with the original altitude once, at zone load, to derive the Barrier layer (per-cell min/max clamp)

Having said that, it would be nice to add a test on the CalculateBarrier. Ex: A small test that builds a TerraformableAltitude from hand-made arrays, assert Barrier min/max on a few cells. This would guard the barrier clamp logic that OnUpdating depends on.

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.

3 participants