Fix future-wait callback accumulation on every spin path

[hugogo1998]

future.add_done_callback() appends unconditionally, and every wait path here was calling it fresh on every spin. A single spin_until_future_complete() that takes N spins to finish was registering N callbacks on the future, and when it completes, Future._schedule_or_invoke_done_callbacks() creates one Task per callback. So waiting N times costs O(N) memory and O(N) work at completion, for no reason. Measured on real Humble rclpy at N=5000: 43.8 ms to complete a future carrying 5000 accumulated callbacks, each one becoming its own Task.

#1374 already fixed this for spin_until_future_complete's internal loop, by rerouting it to a private method that doesn't re-add the callback. That's a real fix and it works: 0.31 ms at N=5000 on that path, matching what this change also gets. But spin_once_until_future_complete is public API, and a caller who writes their own wait loop around it directly (while not future.done(): executor.spin_once_until_future_complete(future), a completely reasonable and documented way to use it) still hits the original bug, because that method still adds a fresh lambda on every call. Checked this on real Humble rclpy: looping it directly at N=5000 costs 44.7 ms and 5000 callbacks, basically unchanged from before #1374, because the fix never reached this path.

This registers one stable wake-on-done callback per Executor instance and adds it to a future at most once, checking future._callbacks before appending, through a single _add_wake_on_done() helper used everywhere a future is waited on: spin_until_future_complete, and both SingleThreadedExecutor's and MultiThreadedExecutor's spin_once_until_future_complete. Every spin path gets the same O(1) behavior instead of just one of them.

Measured on real Humble rclpy, N=5000:

path	before	after
spin_until_future_complete	0.31 ms, 1 callback (already fixed)	0.30 ms, 1 callback
direct spin_once_until_future_complete loop	44.7 ms, 5000 callbacks	0.30 ms, 1 callback

Ties the existing fix on the path it already covers, and closes the gap on the one it didn't, 40x-149x faster depending on N, since the leak scales with how many times a caller has to spin.

The wake-on-done semantics are unchanged: the callback's whole job is to break the executor out of its wait, and firing it once is exactly as sufficient as firing it N times, the future still completes with the correct result either way. This isn't a new mechanism, just making the existing one idempotent everywhere it's used, so it doesn't matter whether a caller uses spin_until_future_complete or writes their own loop around spin_once_until_future_complete, either way they get the fix.

Found via an automated perf-optimization pass profiling the future-completion path; the accumulation showed up directly in the profile as O(N) Task creation.

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request