Skip to content

Conversation

@ndr-ds
Copy link
Contributor

@ndr-ds ndr-ds commented Nov 21, 2025

Motivation

Right now we're using FuturesUnordered, which will poll the futures concurrently in the same task.

Proposal

Use JoinSet so we actually can poll these in parallel, using different cores

Test Plan

Benchmarked a network without this, and with this, and with this our cross chain message queue wait time improved by about 10%. This was on a network with 4 worker threads per shard.

Release Plan

  • Nothing to do / These changes follow the usual release cycle.

Copy link
Contributor Author

ndr-ds commented Nov 21, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@ndr-ds ndr-ds requested review from Twey, afck, deuszx and ma2bd November 21, 2025 16:53
@ndr-ds ndr-ds force-pushed the 11-20-forward_cross_chain_messages_for_different_recipients_actually_in_parallel branch from 91ba951 to a51cedc Compare November 21, 2025 21:06
@ndr-ds ndr-ds force-pushed the 11-19-add_cross_chain_queue_wait_time_and_chain_worker_request_queue_wait_time branch from 76aac22 to a497d6d Compare November 21, 2025 21:06
Copy link
Contributor

@deuszx deuszx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the title you say it's for different recipients but I fail to see where it's guaranteed in this code. Can you point it to me?

Copy link
Contributor Author

ndr-ds commented Nov 25, 2025

Wether we initially spawn a task or not is controlled by job_states, which is a HashMap from QueueId to JobState. So stuff that has the same sender and recipient will use the same task.

@ndr-ds ndr-ds force-pushed the 11-20-forward_cross_chain_messages_for_different_recipients_actually_in_parallel branch from a51cedc to c3b8630 Compare November 25, 2025 17:48
@ndr-ds ndr-ds force-pushed the 11-19-add_cross_chain_queue_wait_time_and_chain_worker_request_queue_wait_time branch from a497d6d to 16c588a Compare November 25, 2025 17:48
@ma2bd
Copy link
Contributor

ma2bd commented Nov 25, 2025

Interesting. What is the CPU-heavy operation taking advantage of multiple cores?

Copy link
Contributor Author

ndr-ds commented Nov 26, 2025

Like I mentioned in the description, this wasn't really a bottleneck. Was just a place that I thought could be one. But the improvements were small, so we're probably not even exhausting the 1 core with this, as it just gets slightly faster in parallel.
I'm also open to closing this, if people think it's not worth it :)
I personally think it doesn't hurt 🤷🏻‍♂️ but up to you guys

@ndr-ds ndr-ds force-pushed the 11-19-add_cross_chain_queue_wait_time_and_chain_worker_request_queue_wait_time branch from 16c588a to c209bce Compare November 26, 2025 13:19
@ndr-ds ndr-ds force-pushed the 11-20-forward_cross_chain_messages_for_different_recipients_actually_in_parallel branch from c3b8630 to 553f6b4 Compare November 26, 2025 13:19
@ndr-ds ndr-ds changed the base branch from 11-19-add_cross_chain_queue_wait_time_and_chain_worker_request_queue_wait_time to graphite-base/5005 November 26, 2025 14:07
@ndr-ds ndr-ds force-pushed the 11-20-forward_cross_chain_messages_for_different_recipients_actually_in_parallel branch from 553f6b4 to 60d57ef Compare November 26, 2025 14:09
@ndr-ds ndr-ds force-pushed the graphite-base/5005 branch from c209bce to b5d9bcd Compare November 26, 2025 14:09
@ndr-ds ndr-ds changed the base branch from graphite-base/5005 to main November 26, 2025 14:09
@ndr-ds ndr-ds requested a review from deuszx November 27, 2025 12:49
Copy link
Contributor

@ma2bd ma2bd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found the answer to my question: it's not about CPU. It's about the select! branches preventing other tasks from being polled.

@ndr-ds ndr-ds added this pull request to the merge queue Dec 1, 2025
Merged via the queue into main with commit 0207db7 Dec 1, 2025
74 of 140 checks passed
@ndr-ds ndr-ds deleted the 11-20-forward_cross_chain_messages_for_different_recipients_actually_in_parallel branch December 1, 2025 13:12
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.

4 participants