Skip to content

Conversation

@frankmcsherry
Copy link
Member

@frankmcsherry frankmcsherry commented Feb 11, 2025

This PR adds a broadcast method to TcpAllocator's allocator implementation, such that it only sends one copy of each broadcast message to each remote process. This uses a modification to MessageHeader to allow for a range of target worker identifiers, on the send side, and the ability to clone Bytes on the receive side.

The change to MessageHeader to allow a range of identifiers feels a bit weird, but seemed the most direct path to a broadcast channel, and not obviously the worst of many other ways we might have indicated the same. If we learn more about sending to subsets of remote workers, or need to economize on message header size, we can revisit!

This should result in a factor #workers/#processes reduction in progress traffic sent between processes.

@frankmcsherry
Copy link
Member Author

frankmcsherry commented Feb 11, 2025

Tested examples/barrier.rs with two process, locally. Shouldn't really be "network bound", but I was able to track the bytes sent and time taken. The time taken is no better, maybe "worse" though I'm not inclined to believe that it is statistically significantly worse.

-w  -n  bytes       bytes/w     bytes/w^2   time     version
1   2   152088      152088      152088      0.606    alt
        152088      152088                  1.035    neu
16  2   38934528    2433408     152088      0.688    alt
        2433408     152088                  0.724    neu
128 2   2491809792  19467264    152088      16.158   alt
        19467264    152088                  18.225   neu

Could still be, of course, but the noise in 256 workers on a laptop is .. pretty understandable.

Copy link
Member

@antiguru antiguru left a comment

Choose a reason for hiding this comment

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

Looks good, thank you.

@frankmcsherry frankmcsherry merged commit 95ee597 into TimelyDataflow:master Feb 12, 2025
7 checks passed
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.

2 participants