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
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 7 additions & 18 deletions cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use powdr_number::{Bn254Field, FieldElement, GoldilocksField};
use powdr_pipeline::{Pipeline, Stage};
use powdr_riscv::continuations::{rust_continuations, rust_continuations_dry_run};
use powdr_riscv::{compile_riscv_asm, compile_rust};
use std::io::{self, BufReader, BufWriter, Read};
use std::io::{self, BufWriter};
use std::path::PathBuf;
use std::{borrow::Cow, fs, io::Write, path::Path};
use strum::{Display, EnumString, EnumVariantNames};
Expand All @@ -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

})
.unwrap_or_default();

Expand Down Expand Up @@ -632,9 +631,7 @@ fn verification_key<T: FieldElement>(
}

fn write_verification_key_to_fs(vkey: Vec<u8>, output_dir: &Path) {
let mut vkey_file = fs::File::create(output_dir.join("vkey.bin")).unwrap();
let mut vkey_writer = BufWriter::new(&mut vkey_file);
vkey_writer.write_all(&vkey).unwrap();
fs::write(output_dir.join("vkey.bin"), vkey).unwrap();
log::info!("Wrote vkey.bin.");
}

Expand All @@ -646,10 +643,9 @@ fn setup<F: FieldElement>(size: u64, dir: String, backend_type: BackendType) {
}

fn write_backend_to_fs<F: FieldElement>(be: &dyn Backend<F>, output_dir: &Path) {
let mut params_file = fs::File::create(output_dir.join("params.bin")).unwrap();
let mut params_writer = BufWriter::new(&mut params_file);
be.write_setup(&mut params_writer).unwrap();
params_writer.flush().unwrap();
let mut params_file = BufWriter::new(fs::File::create(output_dir.join("params.bin")).unwrap());
be.write_setup(&mut params_file).unwrap();
params_file.flush().unwrap();
log::info!("Wrote params.bin.");
}

Expand Down Expand Up @@ -843,14 +839,7 @@ fn read_and_verify<T: FieldElement>(
let proof = Path::new(&proof);
let vkey = Path::new(&vkey).to_path_buf();

let proof = {
let mut buf = Vec::new();
fs::File::open(proof)
.unwrap()
.read_to_end(&mut buf)
.unwrap();
buf
};
let proof = fs::read(proof).unwrap();

let mut pipeline = Pipeline::<T>::default()
.from_file(file.to_path_buf())
Expand Down
4 changes: 2 additions & 2 deletions number/src/serialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ impl Default for CsvRenderMode {
const ROW_NAME: &str = "Row";

pub fn write_polys_csv_file<T: FieldElement>(
file: &mut impl Write,
file: impl Write,
render_mode: CsvRenderMode,
polys: &[&(String, Vec<T>)],
) {
Expand Down Expand Up @@ -53,7 +53,7 @@ pub fn write_polys_csv_file<T: FieldElement>(
writer.flush().unwrap();
}

pub fn read_polys_csv_file<T: FieldElement>(file: &mut impl Read) -> Vec<(String, Vec<T>)> {
pub fn read_polys_csv_file<T: FieldElement>(file: impl Read) -> Vec<(String, Vec<T>)> {
let mut reader = Reader::from_reader(file);
let headers = reader.headers().unwrap();

Expand Down
5 changes: 3 additions & 2 deletions pipeline/build.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::env;
use std::fs::File;
use std::io::Write;
use std::io::{BufWriter, Write};
use std::path::Path;

use walkdir::WalkDir;
Expand All @@ -14,7 +14,7 @@ fn main() {
fn build_book_tests(kind: &str) {
let out_dir = env::var("OUT_DIR").unwrap();
let destination = Path::new(&out_dir).join(format!("{kind}_book_tests.rs"));
let mut test_file = File::create(destination).unwrap();
let mut test_file = BufWriter::new(File::create(destination).unwrap());

let dir = format!("../test_data/{kind}/book/");
for file in WalkDir::new(&dir) {
Expand Down Expand Up @@ -43,4 +43,5 @@ fn {test_name}() {{
.unwrap();
}
}
test_file.flush().unwrap();
}
53 changes: 21 additions & 32 deletions pipeline/src/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::{
borrow::Borrow,
fmt::Display,
fs,
io::{BufWriter, Read, Write},
io::{BufReader, BufWriter, Write},
marker::Send,
path::{Path, PathBuf},
rc::Rc,
Expand Down Expand Up @@ -596,23 +596,21 @@ impl<T: FieldElement> Pipeline<T> {
.expect("backend must be set before calling proving!");
let factory = backend.factory::<T>();
let mut backend = if let Some(path) = self.arguments.setup_file.as_ref() {
let mut file = fs::File::open(path).unwrap();
let mut file = BufReader::new(fs::File::open(path).unwrap());
factory.create_from_setup(&mut file).unwrap()
} else {
factory.create(pil.degree())
};

if let Some(ref path) = self.arguments.vkey_file {
let mut buf = Vec::new();
fs::File::open(path).unwrap().read_to_end(&mut buf).unwrap();
backend.add_verification_key(&pil, &fixed_cols, buf)
backend.add_verification_key(&pil, &fixed_cols, fs::read(path).unwrap())
}

let existing_proof = self.arguments.existing_proof_file.as_ref().map(|path| {
let mut buf = Vec::new();
fs::File::open(path).unwrap().read_to_end(&mut buf).unwrap();
buf
});
let existing_proof = self
.arguments
.existing_proof_file
.as_ref()
.map(|path| fs::read(path).unwrap());

// Even if we don't have all constants and witnesses, some backends will
// still output the constraint serialization.
Expand Down Expand Up @@ -681,10 +679,9 @@ impl<T: FieldElement> Pipeline<T> {

fn maybe_write_constants(&self, constants: &[(String, Vec<T>)]) -> Result<(), Vec<String>> {
if let Some(path) = self.path_if_should_write(|name| format!("{name}_constants.bin"))? {
write_polys_file(
&mut BufWriter::new(&mut fs::File::create(path).unwrap()),
constants,
);
let mut file = BufWriter::new(fs::File::create(path).unwrap());
write_polys_file(&mut file, constants);
file.flush().unwrap();
}
Ok(())
}
Expand All @@ -696,10 +693,9 @@ impl<T: FieldElement> Pipeline<T> {
) -> Result<(), Vec<String>> {
if let Some(witness) = witness.as_ref() {
if let Some(path) = self.path_if_should_write(|name| format!("{name}_commits.bin"))? {
write_polys_file(
&mut BufWriter::new(&mut fs::File::create(path).unwrap()),
witness,
);
let mut file = BufWriter::new(fs::File::create(path).unwrap());
write_polys_file(&mut file, witness);
file.flush().unwrap();
}
}

Expand All @@ -713,10 +709,8 @@ impl<T: FieldElement> Pipeline<T> {
})
.collect::<Vec<_>>();

let mut csv_file = fs::File::create(path).map_err(|e| vec![format!("{}", e)])?;
let mut csv_writer = BufWriter::new(&mut csv_file);

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

}
}

Expand All @@ -728,9 +722,7 @@ impl<T: FieldElement> Pipeline<T> {
if let Some(path) =
self.path_if_should_write(|name| format!("{name}_constraints.json"))?
{
let mut file = fs::File::create(path).unwrap();
file.write_all(constraints_serialization.as_bytes())
.unwrap();
fs::write(path, constraints_serialization.as_bytes()).unwrap();
}
}
if let Some(proof) = &proof_result.proof {
Expand All @@ -740,8 +732,7 @@ impl<T: FieldElement> Pipeline<T> {
"proof.bin"
};
if let Some(path) = self.path_if_should_write(|name| format!("{name}_{fname}"))? {
let mut proof_file = fs::File::create(path).unwrap();
proof_file.write_all(proof).unwrap();
fs::write(path, proof).unwrap();
}
}

Expand Down Expand Up @@ -875,7 +866,7 @@ impl<T: FieldElement> Pipeline<T> {
.expect("backend must be set before generating verification key!");
let factory = backend.factory::<T>();
let backend = if let Some(path) = self.arguments.setup_file.as_ref() {
let mut file = fs::File::open(path).unwrap();
let mut file = BufReader::new(fs::File::open(path).unwrap());
factory.create_from_setup(&mut file).unwrap()
} else {
factory.create(pil.degree())
Expand All @@ -902,16 +893,14 @@ impl<T: FieldElement> Pipeline<T> {
let factory = backend.factory::<T>();

let mut backend = if let Some(path) = self.arguments.setup_file.as_ref() {
let mut file = fs::File::open(path).unwrap();
let mut file = BufReader::new(fs::File::open(path).unwrap());
factory.create_from_setup(&mut file).unwrap()
} else {
panic!("Setup should have been provided for verification")
};

if let Some(ref path) = self.arguments.vkey_file {
let mut buf = Vec::new();
fs::File::open(path).unwrap().read_to_end(&mut buf).unwrap();
backend.add_verification_key(pil, fixed_cols, buf)
backend.add_verification_key(pil, fixed_cols, fs::read(path).unwrap())
} else {
panic!("Verification key should have been provided for verification")
}
Expand Down
13 changes: 6 additions & 7 deletions pipeline/src/test_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ pub fn gen_estark_proof(file_name: &str, inputs: Vec<GoldilocksField>) {

#[cfg(feature = "halo2")]
pub fn gen_halo2_proof(file_name: &str, inputs: Vec<Bn254Field>) {
use std::fs;

let file_name = format!("{}/../test_data/{file_name}", env!("CARGO_MANIFEST_DIR"));
let tmp_dir = mktemp::Temp::new_dir().unwrap();
let mut pipeline = Pipeline::default()
Expand All @@ -93,16 +95,13 @@ pub fn gen_halo2_proof(file_name: &str, inputs: Vec<Bn254Field>) {

// Setup
let setup_file_path = tmp_dir.as_path().join("params.bin");
let mut setup_file = File::create(&setup_file_path).unwrap();
let mut setup_writer = BufWriter::new(&mut setup_file);
backend.write_setup(&mut setup_writer).unwrap();
setup_writer.flush().unwrap();
let mut setup_file = BufWriter::new(File::create(&setup_file_path).unwrap());
backend.write_setup(&mut setup_file).unwrap();
setup_file.flush().unwrap();

// Verification Key
let vkey_file_path = tmp_dir.as_path().join("verification_key.bin");
let vkey = pipeline.verification_key().unwrap();
let mut vkey_file = File::create(&vkey_file_path).unwrap();
vkey_file.write_all(&vkey).unwrap();
fs::write(&vkey_file_path, pipeline.verification_key().unwrap()).unwrap();

// Create the proof before adding the setup and vkey to the backend,
// so that they're generated during the proof
Expand Down
4 changes: 3 additions & 1 deletion riscv/build.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::env;
use std::fs::read_dir;
use std::fs::File;
use std::io::BufWriter;
use std::io::Write;
use std::path::Path;

Expand All @@ -22,7 +23,7 @@ fn build_lalrpop() {
fn build_instruction_tests() {
let out_dir = env::var("OUT_DIR").unwrap();
let destination = Path::new(&out_dir).join("instruction_tests.rs");
let mut test_file = File::create(destination).unwrap();
let mut test_file = BufWriter::new(File::create(destination).unwrap());

let generated_path = "./tests/instruction_tests/generated";
println!("cargo:rerun-if-changed={generated_path}");
Expand Down Expand Up @@ -50,4 +51,5 @@ fn {file_name}() {{
.unwrap();
}
}
test_file.flush().unwrap();
}
Loading