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.

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

Measure execution times of streams #214

Merged
merged 12 commits into from
Aug 8, 2024
Merged

Measure execution times of streams #214

merged 12 commits into from
Aug 8, 2024

Conversation

philsippl
Copy link
Contributor

Measures and aggregates execution times of GPU workload. This is useful for benchmarking and production monitoring.

TODO (followup PR): send this to datadog as a gauge metric

@philsippl philsippl requested review from naure and dkales August 7, 2024 20:56
Comment on lines 575 to 591
// drop actor handle to initiate shutdown
drop(handle);

println!(
"Total time for {} iterations: {:?}",
N_BATCHES - 1,
total_time.elapsed() - batch_times
);

// Clean up server tasks, then wait for them to finish
background_tasks.abort_all();
tokio::time::sleep(Duration::from_secs(5)).await;

// Check for background task hangs and shutdown panics
background_tasks.check_tasks_finished();

Ok(())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Idk if we need a "clean" shutdown for the Server actor, by putting the loop in a try able block and doing the shutdown here on error?

$manager.record_event($streams, &evt0);
$block
$manager.record_event($streams, &evt1);
$map.insert($label, vec![evt0, evt1]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

do you want something like $map.entry($label).or_default().extend(&[evt0, evt1]). I think atm you override the dot_events in each batch iteration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh right, mistake

Comment on lines 821 to 822
.windows(2)
.step_by(2)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is simulating chunks(2)?

@dkales
Copy link
Collaborator

dkales commented Aug 8, 2024

I also touched a few of the tracings in #218 so there are some conflicts

@philsippl philsippl requested a review from dkales August 8, 2024 09:47
@philsippl philsippl merged commit 45462c3 into main Aug 8, 2024
9 checks passed
@philsippl philsippl deleted the ps/log-timing branch August 8, 2024 10:32
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