Skip to content

Commit

Permalink
perf: Read debug info in streaming / chunks
Browse files Browse the repository at this point in the history
Rather than allocating space to store it all in memory. This was done
like this before as during prototyping I did not know if reqwest would
also do this or stream the file.

I've since memory profiled reqwest and checked its source code and it
doesn't keep the whole body in memory.

Test Plan
=========

Manually checked that it continues to work and that the file is read in
chunks with a memory profiler (heaptrack).
  • Loading branch information
javierhonduco committed Jan 16, 2025
1 parent 12eb4f1 commit 474cbbc
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 15 deletions.
27 changes: 14 additions & 13 deletions src/debug_info.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use std::io::Read;
use std::fs::File;
use std::io::BufReader;
use std::path::Path;
use std::path::PathBuf;
use std::time::Duration;

Expand All @@ -20,7 +22,7 @@ pub trait DebugInfoManager {
name: &str,
build_id: &BuildId,
executable_id: ExecutableId,
file: &mut std::fs::File,
debug_info: &Path,
) -> anyhow::Result<()>;
fn debug_info_path(&self) -> Option<PathBuf>;
}
Expand All @@ -32,7 +34,7 @@ impl DebugInfoManager for DebugInfoBackendNull {
_name: &str,
_build_id: &BuildId,
_executable_id: ExecutableId,
_file: &mut std::fs::File,
_debug_info: &Path,
) -> anyhow::Result<()> {
Ok(())
}
Expand All @@ -53,14 +55,14 @@ impl DebugInfoManager for DebugInfoBackendFilesystem {
_name: &str,
build_id: &BuildId,
executable_id: ExecutableId,
file: &mut std::fs::File,
debug_info: &Path,
) -> anyhow::Result<()> {
// try to find, else extract
if self.find_in_fs(build_id) {
return Ok(());
}

self.add_to_fs(build_id, executable_id, file)
self.add_to_fs(build_id, executable_id, debug_info)
}

fn debug_info_path(&self) -> Option<PathBuf> {
Expand All @@ -77,13 +79,14 @@ impl DebugInfoBackendFilesystem {
&self,
build_id: &BuildId,
_executable_id: ExecutableId,
file: &mut std::fs::File,
debug_info: &Path,
) -> anyhow::Result<()> {
// TODO: add support for other methods beyond copying. For example
// hardlinks could be used and only fall back to copying if the src
// and dst filesystems differ.
let mut reader = BufReader::new(File::open(debug_info)?);
let mut writer = std::fs::File::create(self.path.join(build_id.to_string()))?;
std::io::copy(file, &mut writer)?;
std::io::copy(&mut reader, &mut writer)?;
Ok(())
}
}
Expand All @@ -100,7 +103,7 @@ impl DebugInfoManager for DebugInfoBackendRemote {
name: &str,
build_id: &BuildId,
executable_id: ExecutableId,
file: &mut std::fs::File,
debug_info: &Path,
) -> anyhow::Result<()> {
// TODO: add a local cache to not have to reach to the backend
// unnecessarily.
Expand All @@ -109,7 +112,7 @@ impl DebugInfoManager for DebugInfoBackendRemote {
}

// TODO: do this in another thread.
self.upload_to_backend(name, build_id, executable_id, file)?;
self.upload_to_backend(name, build_id, executable_id, debug_info)?;
Ok(())
}

Expand Down Expand Up @@ -142,12 +145,10 @@ impl DebugInfoBackendRemote {
name: &str,
build_id: &BuildId,
executable_id: ExecutableId,
file: &mut std::fs::File,
debug_info: &Path,
) -> anyhow::Result<()> {
let client_builder = reqwest::blocking::Client::builder().timeout(self.http_client_timeout);
let client = client_builder.build()?;
let mut debug_info = Vec::new();
file.read_to_end(&mut debug_info)?;

let response = client
.post(format!(
Expand All @@ -157,7 +158,7 @@ impl DebugInfoBackendRemote {
build_id,
executable_id
))
.body(debug_info)
.body(File::open(debug_info)?)
.send()?;
println!("wrote debug info to server {:?}", response);
Ok(())
Expand Down
4 changes: 2 additions & 2 deletions src/profiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1553,7 +1553,7 @@ impl Profiler {

// We want to open the file as quickly as possible to minimise the chances of races
// if the file is deleted.
let mut file = match fs::File::open(&abs_path) {
let file = match fs::File::open(&abs_path) {
Ok(f) => f,
Err(e) => {
debug!("failed to open file {} due to {:?}", abs_path, e);
Expand Down Expand Up @@ -1628,7 +1628,7 @@ impl Profiler {
&name,
&build_id,
executable_id,
&mut file,
&abs_path,
);
debug!("debug info manager add result {:?}", res);
} else {
Expand Down

0 comments on commit 474cbbc

Please sign in to comment.