From b4ba2f0bf469da7a5fea38f2ef2a9bd069736eba Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 1 May 2023 17:09:59 +1000 Subject: [PATCH 1/3] Change rlink serialization from `MemEncoder` to `FileEncoder`. Because we're writing to a file, so `FileEncoder` is better because we don't have to write all the data to memory first. --- compiler/rustc_codegen_ssa/src/lib.rs | 10 +++++++--- compiler/rustc_interface/src/queries.rs | 3 +-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/lib.rs b/compiler/rustc_codegen_ssa/src/lib.rs index 26d55618b490a..c3cc17c255b46 100644 --- a/compiler/rustc_codegen_ssa/src/lib.rs +++ b/compiler/rustc_codegen_ssa/src/lib.rs @@ -31,7 +31,7 @@ use rustc_middle::dep_graph::WorkProduct; use rustc_middle::middle::dependency_format::Dependencies; use rustc_middle::middle::exported_symbols::SymbolExportKind; use rustc_middle::ty::query::{ExternProviders, Providers}; -use rustc_serialize::opaque::{MemDecoder, MemEncoder}; +use rustc_serialize::opaque::{FileEncoder, MemDecoder}; use rustc_serialize::{Decodable, Decoder, Encodable, Encoder}; use rustc_session::config::{CrateType, OutputFilenames, OutputType, RUST_CGU_EXT}; use rustc_session::cstore::{self, CrateSource}; @@ -39,6 +39,7 @@ use rustc_session::utils::NativeLibKind; use rustc_span::symbol::Symbol; use rustc_span::DebuggerVisualizerFile; use std::collections::BTreeSet; +use std::io; use std::path::{Path, PathBuf}; pub mod back; @@ -215,8 +216,11 @@ const RLINK_MAGIC: &[u8] = b"rustlink"; const RUSTC_VERSION: Option<&str> = option_env!("CFG_VERSION"); impl CodegenResults { - pub fn serialize_rlink(codegen_results: &CodegenResults) -> Vec { - let mut encoder = MemEncoder::new(); + pub fn serialize_rlink( + rlink_file: &Path, + codegen_results: &CodegenResults, + ) -> Result { + let mut encoder = FileEncoder::new(rlink_file)?; encoder.emit_raw_bytes(RLINK_MAGIC); // `emit_raw_bytes` is used to make sure that the version representation does not depend on // Encoder's inner representation of `u32`. diff --git a/compiler/rustc_interface/src/queries.rs b/compiler/rustc_interface/src/queries.rs index 77fbbf64a0ad2..6483d51a0b9a9 100644 --- a/compiler/rustc_interface/src/queries.rs +++ b/compiler/rustc_interface/src/queries.rs @@ -368,9 +368,8 @@ impl Linker { } if sess.opts.unstable_opts.no_link { - let encoded = CodegenResults::serialize_rlink(&codegen_results); let rlink_file = self.prepare_outputs.with_extension(config::RLINK_EXT); - std::fs::write(&rlink_file, encoded) + CodegenResults::serialize_rlink(&rlink_file, &codegen_results) .map_err(|error| sess.emit_fatal(FailedWritingFile { path: &rlink_file, error }))?; return Ok(()); } From 8d359e4385052f012d8d0c2e57a0bcfe54462d44 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 2 May 2023 11:01:58 +1000 Subject: [PATCH 2/3] Move some `Encodable`/`Decodable` tests. Round-trip encoding/decoding of many types is tested in `compiler/rustc_serialize/tests/opaque.rs`. There is also a small amount of encoding/decoding testing in three files in `tests/ui-fulldeps`. There is no obvious reason why these three files are necessary. They were originally added in 2014. Maybe it wasn't possible for a proc macro to run in a unit test back then? This commit just moves the testing from those three files into the unit test. --- compiler/rustc_serialize/tests/opaque.rs | 39 ++++++++++++++++ .../deriving-encodable-decodable-box.rs | 34 -------------- ...riving-encodable-decodable-cell-refcell.rs | 44 ------------------- tests/ui-fulldeps/issue-14021.rs | 33 -------------- 4 files changed, 39 insertions(+), 111 deletions(-) delete mode 100644 tests/ui-fulldeps/deriving-encodable-decodable-box.rs delete mode 100644 tests/ui-fulldeps/deriving-encodable-decodable-cell-refcell.rs delete mode 100644 tests/ui-fulldeps/issue-14021.rs diff --git a/compiler/rustc_serialize/tests/opaque.rs b/compiler/rustc_serialize/tests/opaque.rs index 5e7dd18aa8408..7a7db99b168e6 100644 --- a/compiler/rustc_serialize/tests/opaque.rs +++ b/compiler/rustc_serialize/tests/opaque.rs @@ -251,3 +251,42 @@ fn test_tuples() { check_round_trip(vec![(1234567isize, 100000000000000u64, 99999999999999i64)]); check_round_trip(vec![(String::new(), "some string".to_string())]); } + +#[test] +fn test_unit_like_struct() { + #[derive(Encodable, Decodable, PartialEq, Debug)] + struct UnitLikeStruct; + + check_round_trip(vec![UnitLikeStruct]); +} + +#[test] +fn test_box() { + #[derive(Encodable, Decodable, PartialEq, Debug)] + struct A { + foo: Box<[bool]>, + } + + let obj = A { foo: Box::new([true, false]) }; + check_round_trip(vec![obj]); +} + +#[test] +fn test_cell() { + use std::cell::{Cell, RefCell}; + + #[derive(Encodable, Decodable, PartialEq, Debug)] + struct A { + baz: isize, + } + + #[derive(Encodable, Decodable, PartialEq, Debug)] + struct B { + foo: Cell, + bar: RefCell, + } + + let obj = B { foo: Cell::new(true), bar: RefCell::new(A { baz: 2 }) }; + check_round_trip(vec![obj]); +} + diff --git a/tests/ui-fulldeps/deriving-encodable-decodable-box.rs b/tests/ui-fulldeps/deriving-encodable-decodable-box.rs deleted file mode 100644 index 1c376f59e5174..0000000000000 --- a/tests/ui-fulldeps/deriving-encodable-decodable-box.rs +++ /dev/null @@ -1,34 +0,0 @@ -// run-pass - -#![allow(unused_imports)] -#![feature(rustc_private)] - -extern crate rustc_macros; -extern crate rustc_serialize; - -// Necessary to pull in object code as the rest of the rustc crates are shipped only as rmeta -// files. -#[allow(unused_extern_crates)] -extern crate rustc_driver; - -use rustc_macros::{Decodable, Encodable}; -use rustc_serialize::opaque::{MemDecoder, MemEncoder}; -use rustc_serialize::{Decodable, Encodable, Encoder}; - -#[derive(Encodable, Decodable)] -struct A { - foo: Box<[bool]>, -} - -fn main() { - let obj = A { foo: Box::new([true, false]) }; - - let mut encoder = MemEncoder::new(); - obj.encode(&mut encoder); - let data = encoder.finish(); - - let mut decoder = MemDecoder::new(&data, 0); - let obj2 = A::decode(&mut decoder); - - assert_eq!(obj.foo, obj2.foo); -} diff --git a/tests/ui-fulldeps/deriving-encodable-decodable-cell-refcell.rs b/tests/ui-fulldeps/deriving-encodable-decodable-cell-refcell.rs deleted file mode 100644 index 844d40f2ecd6a..0000000000000 --- a/tests/ui-fulldeps/deriving-encodable-decodable-cell-refcell.rs +++ /dev/null @@ -1,44 +0,0 @@ -// run-pass - -#![allow(unused_imports)] -// This briefly tests the capability of `Cell` and `RefCell` to implement the -// `Encodable` and `Decodable` traits via `#[derive(Encodable, Decodable)]` -#![feature(rustc_private)] - -extern crate rustc_macros; -extern crate rustc_serialize; - -// Necessary to pull in object code as the rest of the rustc crates are shipped only as rmeta -// files. -#[allow(unused_extern_crates)] -extern crate rustc_driver; - -use rustc_macros::{Decodable, Encodable}; -use rustc_serialize::opaque::{MemDecoder, MemEncoder}; -use rustc_serialize::{Decodable, Encodable, Encoder}; -use std::cell::{Cell, RefCell}; - -#[derive(Encodable, Decodable)] -struct A { - baz: isize, -} - -#[derive(Encodable, Decodable)] -struct B { - foo: Cell, - bar: RefCell, -} - -fn main() { - let obj = B { foo: Cell::new(true), bar: RefCell::new(A { baz: 2 }) }; - - let mut encoder = MemEncoder::new(); - obj.encode(&mut encoder); - let data = encoder.finish(); - - let mut decoder = MemDecoder::new(&data, 0); - let obj2 = B::decode(&mut decoder); - - assert_eq!(obj.foo.get(), obj2.foo.get()); - assert_eq!(obj.bar.borrow().baz, obj2.bar.borrow().baz); -} diff --git a/tests/ui-fulldeps/issue-14021.rs b/tests/ui-fulldeps/issue-14021.rs deleted file mode 100644 index 309b5c4a03d57..0000000000000 --- a/tests/ui-fulldeps/issue-14021.rs +++ /dev/null @@ -1,33 +0,0 @@ -// run-pass - -#![allow(unused_mut)] -#![allow(unused_imports)] -#![feature(rustc_private)] - -extern crate rustc_macros; -extern crate rustc_serialize; - -// Necessary to pull in object code as the rest of the rustc crates are shipped only as rmeta -// files. -#[allow(unused_extern_crates)] -extern crate rustc_driver; - -use rustc_macros::{Decodable, Encodable}; -use rustc_serialize::opaque::{MemDecoder, MemEncoder}; -use rustc_serialize::{Decodable, Encodable, Encoder}; - -#[derive(Encodable, Decodable, PartialEq, Debug)] -struct UnitLikeStruct; - -pub fn main() { - let obj = UnitLikeStruct; - - let mut encoder = MemEncoder::new(); - obj.encode(&mut encoder); - let data = encoder.finish(); - - let mut decoder = MemDecoder::new(&data, 0); - let obj2 = UnitLikeStruct::decode(&mut decoder); - - assert_eq!(obj, obj2); -} From ebee3f8515c6f5189b69ae56919ab5bba934aabe Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 1 May 2023 18:51:05 +1000 Subject: [PATCH 3/3] Remove `MemEncoder`. It's only used in tests. Which is bad, because it means that `FileEncoder` is used in the compiler but isn't used in tests! `tests/opaque.rs` now tests encoding/decoding round-trips via file. Because this is slower than memory, this commit also adjusts the `u16`/`i16` tests so they are more like the `u32`/`i32` tests, i.e. they don't test every possible value. --- Cargo.lock | 1 + compiler/rustc_serialize/Cargo.toml | 1 + compiler/rustc_serialize/src/opaque.rs | 129 +---------------------- compiler/rustc_serialize/tests/opaque.rs | 19 ++-- 4 files changed, 17 insertions(+), 133 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f6ee8157b40ef..bff68df401425 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4059,6 +4059,7 @@ dependencies = [ "indexmap", "rustc_macros", "smallvec", + "tempfile", "thin-vec", ] diff --git a/compiler/rustc_serialize/Cargo.toml b/compiler/rustc_serialize/Cargo.toml index e4dbb8a637cea..6046780685ad8 100644 --- a/compiler/rustc_serialize/Cargo.toml +++ b/compiler/rustc_serialize/Cargo.toml @@ -10,3 +10,4 @@ thin-vec = "0.2.12" [dev-dependencies] rustc_macros = { path = "../rustc_macros" } +tempfile = "3.2" diff --git a/compiler/rustc_serialize/src/opaque.rs b/compiler/rustc_serialize/src/opaque.rs index 0f6e4b329b87e..a2ec318df6d83 100644 --- a/compiler/rustc_serialize/src/opaque.rs +++ b/compiler/rustc_serialize/src/opaque.rs @@ -12,118 +12,14 @@ use std::ptr; // Encoder // ----------------------------------------------------------------------------- -pub struct MemEncoder { - pub data: Vec, -} - -impl MemEncoder { - pub fn new() -> MemEncoder { - MemEncoder { data: vec![] } - } - - #[inline] - pub fn position(&self) -> usize { - self.data.len() - } - - pub fn finish(self) -> Vec { - self.data - } -} - -macro_rules! write_leb128 { - ($enc:expr, $value:expr, $int_ty:ty, $fun:ident) => {{ - const MAX_ENCODED_LEN: usize = $crate::leb128::max_leb128_len::<$int_ty>(); - let old_len = $enc.data.len(); - - if MAX_ENCODED_LEN > $enc.data.capacity() - old_len { - $enc.data.reserve(MAX_ENCODED_LEN); - } - - // SAFETY: The above check and `reserve` ensures that there is enough - // room to write the encoded value to the vector's internal buffer. - unsafe { - let buf = &mut *($enc.data.as_mut_ptr().add(old_len) - as *mut [MaybeUninit; MAX_ENCODED_LEN]); - let encoded = leb128::$fun(buf, $value); - $enc.data.set_len(old_len + encoded.len()); - } - }}; -} - -impl Encoder for MemEncoder { - #[inline] - fn emit_usize(&mut self, v: usize) { - write_leb128!(self, v, usize, write_usize_leb128) - } - - #[inline] - fn emit_u128(&mut self, v: u128) { - write_leb128!(self, v, u128, write_u128_leb128); - } - - #[inline] - fn emit_u64(&mut self, v: u64) { - write_leb128!(self, v, u64, write_u64_leb128); - } - - #[inline] - fn emit_u32(&mut self, v: u32) { - write_leb128!(self, v, u32, write_u32_leb128); - } - - #[inline] - fn emit_u16(&mut self, v: u16) { - self.data.extend_from_slice(&v.to_le_bytes()); - } - - #[inline] - fn emit_u8(&mut self, v: u8) { - self.data.push(v); - } - - #[inline] - fn emit_isize(&mut self, v: isize) { - write_leb128!(self, v, isize, write_isize_leb128) - } - - #[inline] - fn emit_i128(&mut self, v: i128) { - write_leb128!(self, v, i128, write_i128_leb128) - } - - #[inline] - fn emit_i64(&mut self, v: i64) { - write_leb128!(self, v, i64, write_i64_leb128) - } - - #[inline] - fn emit_i32(&mut self, v: i32) { - write_leb128!(self, v, i32, write_i32_leb128) - } - - #[inline] - fn emit_i16(&mut self, v: i16) { - self.data.extend_from_slice(&v.to_le_bytes()); - } - - #[inline] - fn emit_raw_bytes(&mut self, s: &[u8]) { - self.data.extend_from_slice(s); - } -} - pub type FileEncodeResult = Result; /// `FileEncoder` encodes data to file via fixed-size buffer. /// -/// When encoding large amounts of data to a file, using `FileEncoder` may be -/// preferred over using `MemEncoder` to encode to a `Vec`, and then writing the -/// `Vec` to file, as the latter uses as much memory as there is encoded data, -/// while the former uses the fixed amount of memory allocated to the buffer. -/// `FileEncoder` also has the advantage of not needing to reallocate as data -/// is appended to it, but the disadvantage of requiring more error handling, -/// which has some runtime overhead. +/// There used to be a `MemEncoder` type that encoded all the data into a +/// `Vec`. `FileEncoder` is better because its memory use is determined by the +/// size of the buffer, rather than the full length of the encoded data, and +/// because it doesn't need to reallocate memory along the way. pub struct FileEncoder { /// The input buffer. For adequate performance, we need more control over /// buffering than `BufWriter` offers. If `BufWriter` ever offers a raw @@ -645,13 +541,6 @@ impl<'a> Decoder for MemDecoder<'a> { // Specialize encoding byte slices. This specialization also applies to encoding `Vec`s, etc., // since the default implementations call `encode` on their slices internally. -impl Encodable for [u8] { - fn encode(&self, e: &mut MemEncoder) { - Encoder::emit_usize(e, self.len()); - e.emit_raw_bytes(self); - } -} - impl Encodable for [u8] { fn encode(&self, e: &mut FileEncoder) { Encoder::emit_usize(e, self.len()); @@ -675,16 +564,6 @@ impl IntEncodedWithFixedSize { pub const ENCODED_SIZE: usize = 8; } -impl Encodable for IntEncodedWithFixedSize { - #[inline] - fn encode(&self, e: &mut MemEncoder) { - let _start_pos = e.position(); - e.emit_raw_bytes(&self.0.to_le_bytes()); - let _end_pos = e.position(); - debug_assert_eq!((_end_pos - _start_pos), IntEncodedWithFixedSize::ENCODED_SIZE); - } -} - impl Encodable for IntEncodedWithFixedSize { #[inline] fn encode(&self, e: &mut FileEncoder) { diff --git a/compiler/rustc_serialize/tests/opaque.rs b/compiler/rustc_serialize/tests/opaque.rs index 7a7db99b168e6..861091688bb2d 100644 --- a/compiler/rustc_serialize/tests/opaque.rs +++ b/compiler/rustc_serialize/tests/opaque.rs @@ -1,9 +1,10 @@ #![allow(rustc::internal)] use rustc_macros::{Decodable, Encodable}; -use rustc_serialize::opaque::{MemDecoder, MemEncoder}; +use rustc_serialize::opaque::{MemDecoder, FileEncoder}; use rustc_serialize::{Decodable, Encodable}; use std::fmt::Debug; +use std::fs; #[derive(PartialEq, Clone, Debug, Encodable, Decodable)] struct Struct { @@ -27,18 +28,21 @@ struct Struct { } fn check_round_trip< - T: Encodable + for<'a> Decodable> + PartialEq + Debug, + T: Encodable + for<'a> Decodable> + PartialEq + Debug, >( values: Vec, ) { - let mut encoder = MemEncoder::new(); + let tmpfile = tempfile::NamedTempFile::new().unwrap(); + let tmpfile = tmpfile.path(); + + let mut encoder = FileEncoder::new(&tmpfile).unwrap(); for value in &values { Encodable::encode(value, &mut encoder); } + encoder.finish().unwrap(); - let data = encoder.finish(); + let data = fs::read(&tmpfile).unwrap(); let mut decoder = MemDecoder::new(&data[..], 0); - for value in values { let decoded = Decodable::decode(&mut decoder); assert_eq!(value, decoded); @@ -61,7 +65,7 @@ fn test_u8() { #[test] fn test_u16() { - for i in u16::MIN..u16::MAX { + for i in [u16::MIN, 111, 3333, 55555, u16::MAX] { check_round_trip(vec![1, 2, 3, i, i, i]); } } @@ -92,7 +96,7 @@ fn test_i8() { #[test] fn test_i16() { - for i in i16::MIN..i16::MAX { + for i in [i16::MIN, -100, 0, 101, i16::MAX] { check_round_trip(vec![-1, 2, -3, i, i, i, 2]); } } @@ -289,4 +293,3 @@ fn test_cell() { let obj = B { foo: Cell::new(true), bar: RefCell::new(A { baz: 2 }) }; check_round_trip(vec![obj]); } -