Hex layout optimization by xfractalino · Pull Request #6793 · Revolutionary-Games/Thrive

xfractalino · 2026-03-11T12:01:56Z

Brief Description of What This PR Does

Avoiding deep-cloning for the organelle layout and hex layouts in general by using temporary pooled memory for diffs only.

Progress Checklist

Note: before starting this checklist the PR should be marked as non-draft.

PR author has checked that this PR works as intended and doesn't
break existing features:
https://wiki.revolutionarygamesstudio.com/wiki/Testing_Checklist
(this is important as to not waste the time of Thrive team
members reviewing this PR)
Initial code review passed (this and further items should not be checked by the PR author)
Functionality is confirmed working by another person (see above checklist link)
Final code review is passed and code conforms to the
styleguide.

Before merging all CI jobs should finish on this PR without errors, if
there are automatically detected style issues they should be fixed by
the PR author. Merging must follow our
styleguide.

hhyyrylainen · 2026-03-11T12:21:40Z

src/auto-evo/steps/ModifyExistingSpecies.cs

+                        mutation.MutatedSpecies.Organelles.Approve(true);
                    }
+
+                    mutation.MutatedSpecies.Organelles.Approve(false);


Should this be in an else clause? Because now if the if is true above then we first all Approve and then Approve false.

hhyyrylainen · 2026-03-11T12:22:26Z

src/general/HexLayout.cs

        return existingHexes.Contains(item);
    }

+    public void Approve(bool isApproved = true)


Why is this method named Approve? Doesn't calling Approve(false) mean that this is rejected? I think for clarity it would be nicer to have two methods: Approve and Reject without any parameters each.

hhyyrylainen · 2026-03-11T12:22:48Z

src/general/HexLayout.cs

+
+        IEnumerator IEnumerable.GetEnumerator()
+        {
+            return GetEnumerator();


This causes boxing due to the way the enumerator is implemented. Hopefully this is not called much?

No, this shouldn't be called at all.

Maybe a GD.PrintErr warning about the performance implications then? (if in the future this somehow ends up being called)

This also happens in the current HexLayout implementation, by the way, also in the standard GetEnumerator (which is used in 66 places, Rider tells me)

But there's a comment with a TODO about this very issue.

I think this case is different, because this is a struct so any allocated enumerator will have a copy of the struct data and not just a reference to the data. So I kind of think that this is a slightly more problematic situation than a simple lightweight enumerator allocation. But I admit I don't fully know how structs having an enumerator function in them behave.

hhyyrylainen · 2026-03-12T09:01:22Z

src/general/IndividualHexLayout.cs

    public void WriteToArchive(ISArchiveWriter writer)
    {
-        writer.WriteObject(existingHexes);
+        writer.WriteObject(existingHexes.MainHexes);


I'm about to start the performance / memory testing of this, so I'll just comment one more thing: I think we need a safety check here in case the data is shared as it would just cut off a portion when saving, right? So I think all of these updated save methods need a method call that the hex layout can react to and make sure it is not shared in order to save correctly. (it could also print an usage warning that only accepted species should be saved)

But silently just not writing all the data should be avoided.

hhyyrylainen

Trying to run a bunch of auto-evo steps I got this error:

Auto-evo failed with an exception: System.ArgumentException: Destination array was not long enough. Check the destination index, length, and the array's lower bounds. (Parameter 'destinationArray')
   at System.Array.CopyImpl(Array sourceArray, Int32 sourceIndex, Array destinationArray, Int32 destinationIndex, Int32 length, Boolean reliable)
   at System.Array.Copy(Array sourceArray, Int32 sourceIndex, Array destinationArray, Int32 destinationIndex, Int32 length)
   at System.Collections.Generic.Queue`1.SetCapacity(Int32 capacity)
   at System.Collections.Generic.Queue`1.Enqueue(T item)
   at AutoEvoRun.GatherInfo(Queue`1 steps) in /home/hhyyrylainen/Projects/Thrive/src/auto-evo/AutoEvoRun.cs:line 405
   at AutoEvoRun.Step() in /home/hhyyrylainen/Projects/Thrive/src/auto-evo/AutoEvoRun.cs:line 559
   at AutoEvoRun.Run() in /home/hhyyrylainen/Projects/Thrive/src/auto-evo/AutoEvoRun.cs:line 533

Note that I merged master to your branch locally before testing as I wanted to test the combined effect of this + my memory optimization. (though I guess I should merge master to this branch here on github as well to make sure we are testing the same code).

hhyyrylainen · 2026-03-12T09:58:01Z

Here's my initial testing numbers that aren't yet super in-depth:

new times (alive species):

03:00 (76)
02:03 (25)
02:56 (113)
03:53 (87)
06:17 (91)

3.072 GB memory use at the end (top of graph)

old times from master branch (alive species):

01:49 (72)
02:28 (57)
02:33 (114)
08:33 (170)
03:26 (82)

5.120 GB memory use at the end (top of graph), peak likely much more

More data is needed to say for sure what the effect is, but it kind of seems like that memory usage is reduced a bit (though that one massive outlier run probably affected the old memory measurement) but it also kind of seems like processing time might have increased. But we definitely need more measurements. What has your results so far looked like, just to confirm we are seeing similar things?

I'm running 5 worlds to generation 15 as my usual auto-evo speed benchmark in the auto-evo exploring tool (and I'm just taking memory usages at the end for now but I think I might need to swap to running a single world to generation 15/20 and then taking the peak memory usage and fully restarting Thrive between measurements).

…d boxing.

xfractalino · 2026-03-13T13:40:48Z

Trying to run a bunch of auto-evo steps I got this error:

Auto-evo failed with an exception: System.ArgumentException: Destination array was not long enough. Check the destination index, length, and the array's lower bounds. (Parameter 'destinationArray')
   at System.Array.CopyImpl(Array sourceArray, Int32 sourceIndex, Array destinationArray, Int32 destinationIndex, Int32 length, Boolean reliable)
   at System.Array.Copy(Array sourceArray, Int32 sourceIndex, Array destinationArray, Int32 destinationIndex, Int32 length)
   at System.Collections.Generic.Queue`1.SetCapacity(Int32 capacity)
   at System.Collections.Generic.Queue`1.Enqueue(T item)
   at AutoEvoRun.GatherInfo(Queue`1 steps) in /home/hhyyrylainen/Projects/Thrive/src/auto-evo/AutoEvoRun.cs:line 405
   at AutoEvoRun.Step() in /home/hhyyrylainen/Projects/Thrive/src/auto-evo/AutoEvoRun.cs:line 559
   at AutoEvoRun.Run() in /home/hhyyrylainen/Projects/Thrive/src/auto-evo/AutoEvoRun.cs:line 533

Note that I merged master to your branch locally before testing as I wanted to test the combined effect of this + my memory optimization. (though I guess I should merge master to this branch here on github as well to make sure we are testing the same code).

I've never gotten this error, and it seems unrelated, as I didn't make any change there.

hhyyrylainen · 2026-03-13T13:49:15Z

I think that error may have been a fluke / due to some unexpected merge thing. I'll resume testing this next week.

xfractalino · 2026-03-17T11:22:31Z

I did some measurements. Memory usage seems to be roughly the same between master and this branch now with the latest merges from master, although GC pressure and allocations seem to be more predictable on this branch.

Top is on this branch.

Processing times also seem to be roughly the same. Sample size is 10 worlds. y is time taken per generation in seconds, x is the generation number. Note that this is a log-log graph.

hhyyrylainen · 2026-03-17T15:33:04Z

Seems like the difference is now very small.

I guess I have to also test the speed effect now?

I'm not sure what we'll do if the effect is too small to really notice as this does make the layout handling quite a bit more complex and introduces the potential for new bugs...

xfractalino · 2026-03-17T15:40:40Z

I'm not measuring any relevant difference now, indeed. If you don't find any difference either then idk, we can simply close this I guess.

hhyyrylainen · 2026-03-18T08:25:18Z

I ran a new set of measurements and it kind of maybe seems like this might have a slight advantage, though way more data would be needed:

Master:
1,19 GB mem at end

- 1:06 (113)
- 0:49 (110)
- 1:03 (171)
- 0:50 (103)
- 0:34 (74)

Hex optimization:
1,19 GB mem at end

- 0:50 (106)
- 0:53 (115)
- 0:39 (113)
- 0:47 (83)
- 0:39 (99)

I think that before any conclusion can be reached on this, we'll need a separate PR merged into master that adds alive species at the end, total processing time, and peak memory usage stats to the worlds export (as talked about on Discord) so that it is possible to do batches like 30 worlds to generation 20 for me (these manual time checks are quite annoying for me to do and don't show a clear enough pattern with this few data points). Without that kind of large amount of data it will be quite hard to tell if this is actually speeding things up measurably.

$@xfractalino$

Hex layout optimization

3d809b5

github-project-automation bot moved this to In progress in Thrive Planning Mar 11, 2026

github-project-automation bot added this to Thrive Planning Mar 11, 2026

$@xfractalino$

Format fixes

c1900d3

hhyyrylainen reviewed Mar 11, 2026

View reviewed changes

$@xfractalino$

Fixed several issues with the implementation

48f85e9

$@xfractalino$ xfractalino force-pushed the hex-layout-optimization branch from 2bcd594 to 48f85e9 Compare March 11, 2026 16:43

hhyyrylainen reviewed Mar 12, 2026

View reviewed changes

hhyyrylainen requested changes Mar 12, 2026

View reviewed changes

Merge branch 'master' into hex-layout-optimization

e550704

xfractalino added 2 commits March 12, 2026 12:26

$@xfractalino$

Added a warning message if the non-generic enumerator is used to avoi…

a721da4

…d boxing.

$@xfractalino$

Custom enumerator implementation

c0aa276

$@xfractalino$ xfractalino force-pushed the hex-layout-optimization branch from 7fb338e to c0aa276 Compare March 13, 2026 12:59

$@xfractalino$

Few style fixes

263fb8c

$@xfractalino$ xfractalino requested a review from hhyyrylainen March 13, 2026 13:41

xfractalino and others added 5 commits March 13, 2026 14:56

$@xfractalino$

Extra safety measures added on layout write to archive

be56a5b

$@xfractalino$

Merge branch 'master' into hex-layout-optimization

f9c64f5

$@xfractalino$

Fixed jetbrains warning

b34a261

$@xfractalino$

Added layout isolation on approval

28ddd6e

$@xfractalino$

Merge branch 'master' into hex-layout-optimization

52f6e03

hhyyrylainen added the review label Mar 17, 2026

hhyyrylainen added this to the Release 1.1.0 milestone Mar 17, 2026

$@xfractalino$

Merge branch 'master' into hex-layout-optimization

4e7872c

Uh oh!

Conversation

xfractalino commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hhyyrylainen left a comment

Choose a reason for hiding this comment

Uh oh!

hhyyrylainen commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xfractalino commented Mar 13, 2026

Uh oh!

hhyyrylainen commented Mar 13, 2026

Uh oh!

xfractalino commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hhyyrylainen commented Mar 17, 2026

Uh oh!

xfractalino commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hhyyrylainen commented Mar 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

$@xfractalino$ xfractalino commented Mar 11, 2026 •

edited

Loading

hhyyrylainen commented Mar 12, 2026 •

edited

Loading

xfractalino commented Mar 17, 2026 •

edited

Loading

xfractalino commented Mar 17, 2026 •

edited

Loading