Skip to content

Improve path finding and radius jewel performance - #10271

Draft
vaisest wants to merge 8 commits into
PathOfBuildingCommunity:devfrom
vaisest:bdap-perf
Draft

Improve path finding and radius jewel performance#10271
vaisest wants to merge 8 commits into
PathOfBuildingCommunity:devfrom
vaisest:bdap-perf

Conversation

@vaisest

@vaisest vaisest commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Description of the problem being solved:

This PR implements BuildAllDependsAndPaths() and other improvements on top of the previous #10228 PR. In my totally realistic synthetic benchmark this result in around a 3x improvement in its performance. This was originally done to make radius jewel comparisons a bit faster, but they are still very slow, so the difference isn't very noticeable there. I think this is more noticeable on the passive tree where this causes less interface lag.

Note that the performance was only tested with one build, which had a light of meaning, lethal pride, and an impossible escape. Still, it definitely shouldn't reduce performance for any build.

The changes include:

  • Caching mastery options (i.e. unallocated mastery info)
  • Using multi-source bfs for closest node calculation
  • Looking up impossible escape keystone instead of scanning every node
  • Avoiding scanning every jewel for each node in spec
  • Generally avoiding useless allocations and other hot path inefficiencies similar to the previous PR

Steps taken to verify a working solution:

  • Tests pass
  • Manually tested features like ascendant pathing which were described in existing comments and code

Link to a build that showcases this PR:

https://poe.ninja/poe1/pob/983a0

@mcagnion

mcagnion commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

A local Codex-assisted analysis found a mastery-option cache regression at cf630666. After removing a Runegraft from an unallocated mastery, RemoveTattooFromNode() restores the base node with an empty sd but leaves allMasteryOptions true. BuildAllDependsAndPaths() then skips rebuilding the mastery options, leaving the hover list empty.

A differential test restores 6 option lines on origin/dev and 0 on this PR. A possible fix is to invalidate allMasteryOptions when removing the Runegraft; the same scenario could also be used as a regression test.

@vaisest
vaisest marked this pull request as draft August 22, 2026 13:08
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.

2 participants