feat: Improve sortBreadcrumbs() performance
#886
Conversation
DafyddLlyr
changed the title
sortBreadcrumbs() performance
There was a problem hiding this comment.
I'm pretty certain that simply timestamping breadcrumbs would radically simplify and speed this up even more - right now we're iterating over tens of thousands of nodes to order tens (max hundreds?) of breadcrumbs.
This is already on our logic camp to-do list, I'll add a Trello card to just pick this one up as a discrete task asap - any implementations of the timestamp can come once we're certain all active flows reliably have them in place.
| const visited = new Set<NodeId>(); | ||
| const stack: string[] = [...(flow._root.edges || [])].reverse(); |
There was a problem hiding this comment.
Same methodology (copy/pasted!) as theopensystemslab/planx-new#5805
| // profile( | ||
| // "Medway RAB template", | ||
| // async () => new DigitalPlanning({ | ||
| // session: mockReportAPlanningBreachSessionMedway, | ||
| // flow: await getMockReportAPlanningBreachFlow(), | ||
| // }) | ||
| // ); No newline at end of file |
There was a problem hiding this comment.
This can be commented back in and ran once #885 has been merged.
jessicamcinchak
left a comment
jessicamcinchak
left a comment
There was a problem hiding this comment.
Great, thanks for further pin-pointing this !
Really tricky / frustrating lately to remember which logic is duplicated in planx-core across different but very similar contexts, but happy this solution was essentially right under our noses from last week 🙈
| "My slow function", | ||
| async () => mySlowFunction(mockData) | ||
| ); | ||
| ``` |
|
Going to merge this into planx-core |
What's the problem?
A recent change in the RAB template flow has lead to a serious slow down in a number of operations which call the
sortBreadcrumbs()function, resulting in timeouts (for example, zip payload generation).Diagnosis
Profiling within the API points to
new DigitalPlanning()as being the bottleneck (~45s of a 50s operation in the example logs below).Further profiling here points towards
sortBreadcrumbs()as they key issue - taking 98% of the total time.What's the solution?
Applying the same lessons as -
dfs(), not recursive call planx-new#5805removeOrphansFromBreadcrumbs()planx-new#5142sortBreadcrumbs()#183The notable change here is simply tracking already visited nodes, and implementing a stack instead of using recursion.