Skip to content

Commit

Permalink
osmet: avoid install performance regression in debug mode on Rust 1.64+
Browse files Browse the repository at this point in the history
Debug builds on Rust 1.64+ were seeing a substantial performance hit in
osmet unpack, causing the kola testiso pxe-offline-install scenario to
time out.  This is due to an apparent Rust performance regression when
BufReader's buffer is larger than the blocks returned by the inner reader,
if the code is built without optimization.  In our case, osmet's
write_unpacked_image() produces data in 8 KiB chunks, which are passed
verbatim through the pipe to the 256 KiB PeekReader in write_image().

For now, work around this by having osmet consolidate writes through a
256 KiB BufWriter.  The extra buffer is harmless, but once the affected
Rust versions are no longer being used for development, it probably makes
sense to drop this code.

xref rust-lang/rust#102727
  • Loading branch information
bgilbert committed Oct 6, 2022
1 parent ed17d9d commit 1ccb9b0
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 2 deletions.
2 changes: 2 additions & 0 deletions docs/release-notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ Major changes:

Minor changes:

- install: Avoid osmet performance regression in debug builds on Rust 1.64+

Internal changes:

Packaging changes:
Expand Down
9 changes: 7 additions & 2 deletions src/osmet/unpacker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

use std::ffi::OsStr;
use std::fs::{File, OpenOptions};
use std::io::{self, copy, ErrorKind, Read, Seek, SeekFrom, Write};
use std::io::{self, copy, BufWriter, ErrorKind, Read, Seek, SeekFrom, Write};
use std::os::unix::ffi::{OsStrExt, OsStringExt};
use std::path::{Path, PathBuf};
use std::thread;
Expand Down Expand Up @@ -107,11 +107,16 @@ fn osmet_unpack_to_writer(
repo: PathBuf,
writer: impl Write,
) -> Result<()> {
let mut w = WriteHasher::new_sha256(writer)?;
// Work around https://github.com/rust-lang/rust/issues/102727 by
// buffering writes into chunks the same size as the buffer
// used by write_image() when reading from us
let buffered = BufWriter::with_capacity(BUFFER_SIZE, writer);
let mut w = WriteHasher::new_sha256(buffered)?;
let n = write_unpacked_image(&mut packed_image, &mut w, &osmet.partitions, &repo)?;
if n != osmet.size {
bail!("wrote {} bytes but expected {}", n, osmet.size);
}
w.flush()?;

let final_checksum: Sha256Digest = w.try_into()?;
if final_checksum != osmet.checksum {
Expand Down

0 comments on commit 1ccb9b0

Please sign in to comment.