Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Activate only by channel ID #526

Merged
merged 3 commits into from
Feb 5, 2024
Merged

Activate only by channel ID #526

merged 3 commits into from
Feb 5, 2024

Conversation

antiguru
Copy link
Member

@antiguru antiguru commented Jun 4, 2023

This changes activation to avoid an associated event, which is currently not used anymore. It also switches to a Vec to contain the activations, and deduplicates before activating operators.

This improves performance in edge cases:

➜  timely-dataflow git:(activation_no_event) time taskset 1 target/release/examples/pingpong $((2**20)) 64
taskset 1 target/release/examples/pingpong $((2**20)) 64  3.63s user 0.00s system 99% cpu 3.638 total
➜  timely-dataflow-master git:(master) time taskset 1 target/release/examples/pingpong $((2**20)) 64
taskset 1 target/release/examples/pingpong $((2**20)) 64  3.32s user 0.78s system 99% cpu 4.106 total

antiguru
antiguru commented Jun 9, 2023
View reviewed changes
communication/src/allocator/counters.rs Outdated
@@ -2,7 +2,6 @@

use std::rc::Rc;
use std::cell::RefCell;
use std::collections::VecDeque;

use crate::{Push, Pull};
use crate::allocator::Event;
Copy link
Member Author

Choose a reason for hiding this comment

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

Check if Event still needs to exist.

timely/src/worker.rs Outdated
for (channel, _event) in borrow.drain(..) {
borrow.sort_unstable();
borrow.dedup();
for channel in borrow.drain(..) {
// TODO: Pay more attent to `_event`.
Copy link
Member Author

Choose a reason for hiding this comment

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

Update comment.

@antiguru
Activate only by channel ID
b4030e9 
Signed-off-by: Moritz Hoffmann <[email protected]>
@antiguru
Remove Event
34f287d 
Signed-off-by: Moritz Hoffmann <[email protected]>
@antiguru
Remove comment
b2ec93f 
Signed-off-by: Moritz Hoffmann <[email protected]>
frankmcsherry
frankmcsherry approved these changes Feb 5, 2024
View reviewed changes
Copy link
Member

@frankmcsherry frankmcsherry left a comment

Choose a reason for hiding this comment

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

This makes sense to me, and we've left the comment in that reminds us how it might be different in the future (tracking the emptiness of channels, and not scheduling only on the basis of a message both sent and received).

@frankmcsherry frankmcsherry merged commit b869dcb into TimelyDataflow:master Feb 5, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@frankmcsherry frankmcsherry frankmcsherry approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@antiguru @frankmcsherry