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

fix off-by-one bug while reading heaptrack files #8101

Merged
merged 4 commits into from
May 16, 2024
Merged

fix off-by-one bug while reading heaptrack files #8101

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
522ecc3
fix off-by-one error
sokra May 7, 2024
03e6f6c
report stats at end of reading
sokra May 7, 2024
39adac6
avoid max to become zero
sokra May 7, 2024
8a791bf
clippy
sokra May 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions crates/turbopack-trace-server/src/reader/heaptrack.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,7 @@ impl HeaptrackFormat {
}
},
trace_instruction_pointers: vec![0],
allocations: vec![AllocationInfo {
size: 0,
trace_index: 0,
}],
allocations: vec![],
spans: 0,
collapse_crates: env::var("COLLAPSE_CRATES")
.unwrap_or_default()
Expand Down
31 changes: 17 additions & 14 deletions crates/turbopack-trace-server/src/reader/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
store_container::StoreContainer,
};

const MIN_INITIAL_REPORT_SIZE: u64 = 100 * 1024 * 1024;

trait TraceFormat {
fn read(&mut self, buffer: &[u8]) -> Result<usize>;
fn stats(&self) -> String {
Expand Down Expand Up @@ -115,17 +117,7 @@
let mut format: Option<Box<dyn TraceFormat>> = None;

let mut current_read = 0;
let mut initial_read = {
if let Ok(pos) = file.seek(SeekFrom::End(0)) {
if pos > 100 * 1024 * 1024 {
Some(pos)
} else {
None
}
} else {
None
}
};
let mut initial_read = { file.seek(SeekFrom::End(0)).ok() };
if file.seek(SeekFrom::Start(0)).is_err() {
return false;
}
Expand All @@ -145,7 +137,9 @@
match file.read(&mut chunk) {
Ok(bytes_read) => {
if bytes_read == 0 {
if let Some(value) = self.wait_for_more_data(&mut file, &mut initial_read) {
if let Some(value) =
self.wait_for_more_data(&mut file, &mut initial_read, format.as_ref())
{
return value;
}
} else {
Expand Down Expand Up @@ -220,7 +214,9 @@
}
Err(err) => {
if err.kind() == io::ErrorKind::UnexpectedEof {
if let Some(value) = self.wait_for_more_data(&mut file, &mut initial_read) {
if let Some(value) =
self.wait_for_more_data(&mut file, &mut initial_read, format.as_ref())
{
return value;
}
} else {
Expand All @@ -237,12 +233,19 @@
&mut self,
file: &mut TraceFile,
initial_read: &mut Option<u64>,
format: Option<&Box<dyn TraceFormat>>,

Check failure on line 236 in crates/turbopack-trace-server/src/reader/mod.rs

View workflow job for this annotation

GitHub Actions / Turbopack rust clippy 

you seem to be trying to use `&Box<T>`. Consider using just `&T`
) -> Option<bool> {
let Ok(pos) = file.stream_position() else {
return Some(true);
};
if let Some(total) = initial_read.take() {
println!("Initial read completed ({} MB)", total / (1024 * 1024),);
if let Some(format) = format {
let stats = format.stats();
println!("{}", stats);
}
if total > MIN_INITIAL_REPORT_SIZE {
println!("Initial read completed ({} MB)", total / (1024 * 1024));
}
}
thread::sleep(Duration::from_millis(100));
let Ok(end) = file.seek(SeekFrom::End(0)) else {
Expand Down
2 changes: 1 addition & 1 deletion crates/turbopack-trace-server/src/viewer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -853,7 +853,7 @@ impl Viewer {
.collect();
Update {
lines,
max: current,
max: max(1, current),
}
}
}
Expand Down
Loading