ActorMap, do not look up influence node up to three times in cell layer.

[anvilvapre]

partial successor of #20351, which will be closed.

• in find free cell, avoid potential 3 times lookup of influence node in influence cell layer, by pasing the head influence node directly.

• do no call function recursively to remove element from list. avoid cell layer cell update if head element did not change.

[Mailaender]

CPU Benchmark

bleed: ⌀ 2.56 ± 1.73 ms
this: ⌀ 3.65 ± 2,41 ms

[anvilvapre]

• in find free cell, avoid potential 3 times lookup of influence node in influence cell layer, by pasing the head influence node directly.

FreeSubCell bleed: 647.4 ticks after 500 calls.
FreeSubCell this pull request: 481.3 ticks after 500 calls.
~25% faster
other functions also take advantage of this change.

• do no call function recursively to remove element from list. avoid cell layer cell update if head element did not change.

This pull request: 50 ticks on avg slower.

Will revert the second.

[Mailaender]

CPU Benchmark

bleed: ⌀ 2.56 ± 1.73 ms
this: ⌀ 3.78 ± 2,59 ms

[Mailaender]

Conceptual concerns regarding my measurements by @abcdefg30.

[anvilvapre]

i noticed that during shell map start up a lot calls to actor map find cell are being made (4000 or so). once the shell map plays it is less (500 each few many seconds). likely related to the orders being processed.

i did look at the source of truetick_time.csv and as i recall it is measured from inner logic tick. so it should include these changes. although i am not sure this change would show a very clear reduction in tick time. only what i don't understand is that it shows that it became slower - based on the changes. unless i introduced some inefficiency that i overlooked.

[RoosterDragon]

Looping the functions 1000x as follows is enough to get it to appear on a sampling profiler. I measured from start until RA shellmap had run for 1000 ticks.

public SubCell FreeSubCell(CPos cell, SubCell preferredSubCell = SubCell.Any, bool checkTransient = true)
{
	for (var i = 0; i < 1000; i++)
	{
		FreeSubCell2(cell, preferredSubCell, checkTransient);
	}

	return FreeSubCell2(cell, preferredSubCell, checkTransient);

	SubCell FreeSubCell2(CPos cell, SubCell preferredSubCell, bool checkTransient)
	{
		// original method body
 	}
}

Method	Bleed	PR
SubCell FreeSubCell(..., bool checkTransient = true)	1.27%	0.44%
SubCell FreeSubCell(...l, Func<Actor, bool> checkIfBlocker)	0.34%	0.17%

Once you undo the 1000x multiplier we're talking about methods that cost 0.002% of the runtime. Therefore we won't see any impact in timing logs. Any changes will be easily swamped in noise.

Since this is a clean and straightforward change I am happy to approve it. In future it would be useful to outline the impact up front to set reviewer expectations. Changes with noticable impact are more likely to get attention that changes with unknown or no impact.

[Mailaender]

CPU Benchmark

bleed: ⌀ 2.64 ± 1.79 ms
this: ⌀ 2.90 ± 2,41 ms

It seems my measurement of the shellmap is not too reproducible.

[Mailaender]

Changelog