Task scheduler and ForestTraversal

[dellaert]

With some help, created one "core" scheduler, specialized using a Policy class to either task traversal or priority (which I need in other places). Some refactoring of the core scheduler to make it faster. One particular change, to drastically decrease the use of condition_.notify_all(); made a 5-fold difference on Linux

Now, on Linux, the new MixIn implementation alone delivers large speedups over classic GTSAM (up to ~26× on large chain problems). While TBB still achieves the lowest absolute MultifrontalSolver times, both backends now show strong performance improvements on the largest BAL and chain benchmarks.

OS	Backend	Problem	Multifrontal (s)	Classic GTSAM (s)	Speedup (×)
macOS	MixIn	BAL dubrovnik-135	3.03961	5.81504	1.91308
macOS	TBB	BAL dubrovnik-135	2.64705	4.07572	1.53972
Linux	MixIn	BAL dubrovnik-135	1.85265	4.47684	2.41646
Linux	TBB	BAL dubrovnik-135	1.78422	2.16964	1.21602
macOS	MixIn	Chain T=5000	0.52494	10.2654	19.5554
macOS	TBB	Chain T=5000	0.465384	7.72200	16.5928
Linux	MixIn	Chain T=5000	0.272993	7.12812	26.1110
Linux	TBB	Chain T=5000	0.214422	4.85482	22.6415

That means that even in the absence of TBB, users should see a big jump in performance - once integrated in NonlinearOptimizer (next next PR :-))

[Copilot]

Pull request overview

This PR introduces a priority-based task scheduler and a forest traversal system that can utilize either the new scheduler or TBB (when available), aimed at improving parallel performance even in non-TBB builds.

• Adds PriorityScheduler<Y> - a thread pool with priority queue scheduling and work stealing
• Adds ForestTraversal - a CRTP mixin providing top-down and bottom-up tree traversal with automatic TBB/scheduler selection
• Includes basic test coverage for both new components

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 21 comments.

Show a summary per file

File	Description
gtsam/base/PriorityScheduler.h	New priority-based task scheduler with per-worker queues and work stealing
gtsam/base/ForestTraversal.h	New forest traversal mixin with TBB and scheduler backends
gtsam/base/tests/testPriorityScheduler.cpp	Basic test for scheduler return value composition
gtsam/base/tests/testForestTraversal.cpp	Basic tests for bottom-up and top-down traversal
gtsam/base/FastSet.h	Removes IsTestable concept assertion (unexplained)
.github/copilot-instructions.md	Adds guideline about public struct field naming

[dellaert]

@copilot open a new pull request to apply changes based on the comments in this thread

[Copilot]

@dellaert I've opened a new pull request, #2344, to work on those changes. Once the pull request is ready, I'll request review from you.

[ProfFan]

Hmm I need a bit of time looking at this code but why do we need priority scheduling here? It seems to me that the work is deterministic and cooperative scheduling is enough. Might be that just the name is misleading?

[dellaert]

Hmm I need a bit of time looking at this code but why do we need priority scheduling here? It seems to me that the work is deterministic and cooperative scheduling is enough. Might be that just the name is misleading?

It’s true, priority is not needed. Even hurts a bit in perf, maybe a %. But I have a PR after these two PRs that fixes that.

[dellaert]

@ProfFan See #2345

[ProfFan]

updateParent is the most time consuming part and is easily parallelizable:
either:

1. change the iteration from by child to by key, and distribute by key
2. divide the child to $n_{core}$ groups and map-reduce.

[dellaert]

@ProfFan uncanny, that’s the next PR :-) it gives another nice improvement, especially for BAL. But can we land this first?

[dellaert]

PS, having 30k children update the same parent sbm_ needed creating per-thread SBM temporaries. If we have to lock parallelism does not help :-)

[ProfFan]

That's why you need deterministic work distribution so threads don't race ;)

[dellaert]

So, are we good to go on this one?

[dellaert]

I’ll fix test in next PR