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

Cleanup on file reads and writes. #1038

Merged
merged 1 commit into from
Feb 9, 2024
Merged

Cleanup on file reads and writes. #1038

merged 1 commit into from
Feb 9, 2024

Conversation

lvella
Copy link
Member

@lvella lvella commented Feb 8, 2024

  • Added buffer in places that didn't have it,
  • removed buffer in places that didn't need it,
  • replaced Vec<u8> reads with fs::read(),
  • replaced &[u8] writes with fs::write().

@@ -32,8 +32,7 @@ fn bind_cli_args<F: FieldElement>(
let witness_values = witness_values
.map(|csv_path| {
let csv_file = fs::File::open(csv_path).unwrap();
let mut csv_writer = BufReader::new(&csv_file);
read_polys_csv_file::<F>(&mut csv_writer)
read_polys_csv_file::<F>(csv_file)
Copy link
Member Author

Choose a reason for hiding this comment

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

csv::Reader has its own buffer, so BufReader is redundant here


write_polys_csv_file(&mut csv_writer, self.arguments.csv_render_mode, &columns);
let csv_file = fs::File::create(path).map_err(|e| vec![format!("{}", e)])?;
write_polys_csv_file(csv_file, self.arguments.csv_render_mode, &columns);
Copy link
Member Author

Choose a reason for hiding this comment

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

csv::Writer has its own buffer, so BufWriter is redundant here

chriseth
chriseth previously approved these changes Feb 9, 2024
chriseth
chriseth previously approved these changes Feb 9, 2024
@lvella lvella added this pull request to the merge queue Feb 9, 2024
Merged via the queue into main with commit f9826a0 Feb 9, 2024
5 checks passed
@lvella lvella deleted the buffered_file_io branch February 9, 2024 12:10
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.

3 participants