Reuse a PathCalculator to substantially speed up routing queries agai…

[dabreegster]

…nst the same CH.

shortcuts in bristol_east
                        time:   [1.2296 ms 1.2569 ms 1.2905 ms]
                        change: [-50.414% -49.842% -49.172%] (p = 0.00 < 0.05)
                        Performance has improved.
shortcuts in bristol_west
                        time:   [3.8934 ms 3.9038 ms 3.9153 ms]
                        change: [-48.807% -47.506% -46.338%] (p = 0.00 < 0.05)
                        Performance has improved.
shortcuts in strasbourg time:   [2.8561 ms 2.8767 ms 2.9002 ms]
                        change: [-85.964% -85.814% -85.657%] (p = 0.00 < 0.05)
                        Performance has improved.

The speedup is enormous -- 50% in Bristol, 85% in Strasbourg. It's roughly proportional to the size of the graph. The path calculator is some massive vector per node in the CH, so constantly allocating that doesn't make sense.

I didn't do this originally, just so we could have the satisfaction of getting the speedup now. ;)

[dabreegster]

Is it OK to do this? I can't imagine Debug was ever too useful; things like FastGraph and NodeMap are enormous unintelligible lists of numbers. And I didn't see anywhere doing Clone. If we want to keep either, we just need a workaround to ignore PathCalculator

[michaelkirk]

I made use of FastGraph's debug representation when debugging the recent routing work. Agreed it's not practical for debugging real world routing, but it's useful enough when you have a tiny number of edges, like in the unit tests I was using to drive development.

I pretty much automatically add common derive's (Debug, Clone, sometimes PartialEq) to structs just in case I (or someone) needs them in the future. As far as I know there's nothing to be gained by being stingy with them, unless they're getting in the way, like this one is now.

It's kind of annoying to have to add them back later when they're in a separate repo, e.g. a-b-street/utils@8a98043

All that said, it's totally fine to remove this Debug impl for now since it's in the way, and revisit if I need to use it again in the future.

[michaelkirk]

Nice speedup!

We may have to revisit if we end up using webworkers/multithreading for our routing computations.

[michaelkirk]

I made use of FastGraph's debug representation when debugging the recent routing work. Agreed it's not practical for debugging real world routing, but it's useful enough when you have a tiny number of edges, like in the unit tests I was using to drive development.

I pretty much automatically add common derive's (Debug, Clone, sometimes PartialEq) to structs just in case I (or someone) needs them in the future. As far as I know there's nothing to be gained by being stingy with them, unless they're getting in the way, like this one is now.

It's kind of annoying to have to add them back later when they're in a separate repo, e.g. a-b-street/utils@8a98043

All that said, it's totally fine to remove this Debug impl for now since it's in the way, and revisit if I need to use it again in the future.

[dabreegster]

We may have to revisit if we end up using webworkers/multithreading for our routing computations.

Thread-local PathCalculators are the way to go then. There has to be a little care when using rayon to only initialize it once per thread, when the work-stealing stuff is enabled. But it's something I've figured out before: https://github.com/Urban-Analytics-Technology-Platform/od2net/blob/186c111f8cb5d006eaa33dfb03fc4faef9a65092/od2net/src/router.rs#L42

The hard part about multithreading will be getting rayon + vite happy, from vague memory...