-
Notifications
You must be signed in to change notification settings - Fork 34
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
feat(hydro_deploy)!: Perf works over SSH #1313
Conversation
hydro_deploy/core/src/ssh.rs
Outdated
runtime | ||
.block_on(session.disconnect(None, "", None)) | ||
.block_on(async move { | ||
self.channel.write_all(&[b'\x03']).await.unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can probably move this into the block if we want perf data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wdym?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only send the Ctrl-C and wait for the program to shutdown if we launched the program with perf; otherwise the default logic of just dropping the connection should be fine.
hydro_deploy/core/src/ssh.rs
Outdated
// Copy local binary to {output_file}.bins/home/{user}/hydro-{unique_name} | ||
let output_file = perf.output_file.to_str().unwrap(); | ||
let local_binary = PathBuf::from(format!( | ||
"{output_file}.bins/home/{user}/hydro-{unique_name}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of duplicating the path here can we concatenate binary_path
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, why are we creating local_binary
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's because the perf.data is linking to a binary it expects at that location (and was there on remote but is not there locally)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, so scripts that process perf data look at that file to find symbols and what not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, so if the file isn't there perf shows a bunch of unlabeled functions
rebased on latest |
b943cba
to
ee2a5c9
Compare
4f84634
to
0e64791
Compare
…a when no perf data was created
See documentation on how to use in Notion.
Fix #1353
BREAKING CHANGE: changes some config arguments