Skip to content

Memoize GetLabel for ~1.5% sim speedup at CPU-bound endgame#7163

Draft
narfman0 wants to merge 2 commits into
FAForever:developfrom
narfman0:perf/navutils-getlabel-memoize
Draft

Memoize GetLabel for ~1.5% sim speedup at CPU-bound endgame#7163
narfman0 wants to merge 2 commits into
FAForever:developfrom
narfman0:perf/navutils-getlabel-memoize

Conversation

@narfman0

@narfman0 narfman0 commented Jul 2, 2026

Copy link
Copy Markdown

Description of the proposed changes

GetLabel re-descends the static navmesh quad-tree on every call. M28AI issues ~558 calls/tick at ~1k units, growing super-linearly with unit count. At CPU-bound endgame (~2.5k units, 4v4 Seton's) this costs ~1.5 ms/tick (~1.5% of a 100 ms tick). Cache it.

Key: floor(x), floor(z) integer ogrids. The quad-tree bottoms out at an integer-aligned compressionThreshold grid, so a floored cell falls inside exactly one leaf with one permanent label. Only permanent labels are cached — transient results (NotGenerated, SystemError) are skipped.

Testing done on the proposed changes

Measured results (4v4 M28AI live to endgame, ~2.5k units):

  • Hit rate: 98.6% at endgame (94% mid-game; startup map scan drags cumulative)
  • Saving: ~0.4% mid-game → ~1.5% at CPU-bound endgame (scales as sim gets heavier)
  • Memory: bounded by queried map area × layers (~10^5 entries, few MB)
  • Determinism: terrain labels are static — no desync risk

Additional context

I spent some time in ghidra, faf repos, m28 repo, and claude code getting some insights here: https://github.com/narfman0/faf-multicore. I have a lot of determinism concerns with a lot of the other work, but this one should be safe, and I tested in practice :)

Checklist

GetLabel re-descends the static navmesh quad-tree on every call. M28AI issues
~558 calls/tick at ~1k units, growing super-linearly with unit count. At
CPU-bound endgame (~2.5k units, 4v4 Seton's) this costs ~1.5 ms/tick (~1.5% of
a 100 ms tick). Cache it.

Key: floor(x), floor(z) integer ogrids. The quad-tree bottoms out at an
integer-aligned compressionThreshold grid, so a floored cell falls inside
exactly one leaf with one permanent label. Only permanent labels are cached —
transient results (NotGenerated, SystemError) are skipped.

Measured results (4v4 M28AI live to endgame, ~2.5k units):
- Hit rate: 98.6% at endgame (94% mid-game; startup map scan drags cumulative)
- Saving: ~0.4% mid-game → ~1.5% at CPU-bound endgame (scales as sim gets heavier)
- Memory: bounded by queried map area × layers (~10^5 entries, few MB)
- Determinism: terrain labels are static — no desync risk
@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 72a12bd0-fd68-490c-8ca6-0f84a4531324

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@narfman0 narfman0 force-pushed the perf/navutils-getlabel-memoize branch from ccbea8c to 2cc71ba Compare July 2, 2026 21:26
@lL1l1 lL1l1 added the area: AI related to AI functions label Jul 8, 2026
@lL1l1

lL1l1 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Is this ready for review?

@Garanas Garanas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In general I am not sure if I understood correct, so please correct me if I am wrong.

Based on my understanding, let's say that we are playing on Seton's Clutch. The heightmap is 1024x1024 in size. We floor the posiiton, which means there are 1024x1024 valid indices.

Technically, for each index we create a new entry in a two dimensional table (table of tables). The inserting of the entries is not in order, meaning most values would not be stored as an index (8 bytes each) but as part of a hash (ceiling to the nearest power of 2, times 20 bytes).

As the game continues, this data structure will grow a lot. You mention in your PR that this could be a few megabytes, which is quite significant. In the worse case this could easily be more than a dozen megabytes, especially when this function is used to scan an area.

The game is limited in memory (32 bits), Lua can't use the full available memory space. When we hit the memory, the game flat out crashes.

In the best case, this data structure would consume (1024x1024x8) + 1024x40 = 8429568 bytes, which is 8.429568mb. When all entries are hashes, you get (1024x1024x20)+ 1024x40 = 21012480 bytes, which is 21.012480mb. This is more than the average navigational mesh will consume of a 20x20 map.

Again, if I understood correct, this is a clear trade off of gaining 1.5ms at the cost of several to dozen(s) of megabytes of memory. I'm not sure if the tradeoff is worth it. I would personally not be in favor.

Comment thread lua/sim/NavUtils.lua
local row = lc[kx]
if not row then
row = {}
lc[kx] = row

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Inserting is essentially arbitrary, which means that kx is not an index (8 bytes) but part of a hash (at least 20 bytes each).

Comment thread lua/sim/NavUtils.lua
if entry ~= nil then
-- cache hit: false encodes a permanent nil result
if entry == false then
return nil, 'CachedNil'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The return value CachedNil does not explain to the caller why there is no label. The point of the last return argument is to help the caller to understand this. For example: you tried to query outside the map or the area is not pathable.

@narfman0

narfman0 commented Jul 9, 2026

Copy link
Copy Markdown
Author

Is this ready for review?

Not yet, I wanted to get my buddy on a laptop to test this first. I intended to polish before promoting from draft, pardon the noise!

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

Labels

area: AI related to AI functions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants