From 88609e51265a563552e8fb4509f83a99e15451b2 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Wed, 2 Mar 2022 18:38:40 +0100 Subject: [PATCH 1/4] Rename derive_merge macro to define_config and move Deserialize impl into it --- src/bootstrap/config.rs | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/src/bootstrap/config.rs b/src/bootstrap/config.rs index 0c0a4733231d2..7e6eecaaa884f 100644 --- a/src/bootstrap/config.rs +++ b/src/bootstrap/config.rs @@ -362,11 +362,13 @@ impl Merge for TomlConfig { // We are using a decl macro instead of a derive proc macro here to reduce the compile time of // rustbuild. -macro_rules! derive_merge { +macro_rules! define_config { ($(#[$attr:meta])* struct $name:ident { $($field:ident: $field_ty:ty,)* }) => { $(#[$attr])* + #[derive(Deserialize)] + #[serde(deny_unknown_fields, rename_all = "kebab-case")] struct $name { $($field: $field_ty,)* } @@ -383,10 +385,9 @@ macro_rules! derive_merge { } } -derive_merge! { +define_config! { /// TOML representation of various global build decisions. - #[derive(Deserialize, Default)] - #[serde(deny_unknown_fields, rename_all = "kebab-case")] + #[derive(Default)] struct Build { build: Option, host: Option>, @@ -429,10 +430,8 @@ derive_merge! { } } -derive_merge! { +define_config! { /// TOML representation of various global install decisions. - #[derive(Deserialize)] - #[serde(deny_unknown_fields, rename_all = "kebab-case")] struct Install { prefix: Option, sysconfdir: Option, @@ -444,10 +443,8 @@ derive_merge! { } } -derive_merge! { +define_config! { /// TOML representation of how the LLVM build is configured. - #[derive(Deserialize)] - #[serde(deny_unknown_fields, rename_all = "kebab-case")] struct Llvm { skip_rebuild: Option, optimize: Option, @@ -479,9 +476,7 @@ derive_merge! { } } -derive_merge! { - #[derive(Deserialize)] - #[serde(deny_unknown_fields, rename_all = "kebab-case")] +define_config! { struct Dist { sign_folder: Option, gpg_password_file: Option, @@ -505,10 +500,8 @@ impl Default for StringOrBool { } } -derive_merge! { +define_config! { /// TOML representation of how the Rust build is configured. - #[derive(Deserialize)] - #[serde(deny_unknown_fields, rename_all = "kebab-case")] struct Rust { optimize: Option, debug: Option, @@ -560,10 +553,8 @@ derive_merge! { } } -derive_merge! { +define_config! { /// TOML representation of how each build target is configured. - #[derive(Deserialize)] - #[serde(deny_unknown_fields, rename_all = "kebab-case")] struct TomlTarget { cc: Option, cxx: Option, From a0b4d2136d639590fa2be32f8b092c5766bc92af Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Wed, 2 Mar 2022 19:43:43 +0100 Subject: [PATCH 2/4] Use trimmed down deserialization impl for config This reduces binary size from 10.1MiB (6.2MiB for just rustbuild code) to 9.7MiB (5.8MiB for just rustbuild code). This also reduces compile time from ~6.1s for incr recompilation to ~5.6s. There is still a lot of unnecessary code due to the toml crate monomorphizing every deserialization impl 5 times. --- src/bootstrap/config.rs | 383 +++++++++++++++++++++++++--------------- 1 file changed, 237 insertions(+), 146 deletions(-) diff --git a/src/bootstrap/config.rs b/src/bootstrap/config.rs index 7e6eecaaa884f..762f18fb32f8e 100644 --- a/src/bootstrap/config.rs +++ b/src/bootstrap/config.rs @@ -17,7 +17,7 @@ use crate::channel::GitInfo; pub use crate::flags::Subcommand; use crate::flags::{Color, Flags}; use crate::util::{exe, t}; -use serde::Deserialize; +use serde::{Deserialize, Deserializer}; macro_rules! check_ci_llvm { ($name:expr) => { @@ -364,13 +364,11 @@ impl Merge for TomlConfig { // rustbuild. macro_rules! define_config { ($(#[$attr:meta])* struct $name:ident { - $($field:ident: $field_ty:ty,)* + $($field:ident: Option<$field_ty:ty> = $field_key:literal,)* }) => { $(#[$attr])* - #[derive(Deserialize)] - #[serde(deny_unknown_fields, rename_all = "kebab-case")] struct $name { - $($field: $field_ty,)* + $($field: Option<$field_ty>,)* } impl Merge for $name { @@ -382,6 +380,99 @@ macro_rules! define_config { )* } } + + // The following is a trimmed version of what serde_derive generates. All parts not relevant + // for toml deserialization have been removed. This reduces the binary size and improves + // compile time of rustbuild. + impl<'de> Deserialize<'de> for $name { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + #[allow(non_camel_case_types)] + enum FieldName { + $($field,)* + } + struct FieldNameVisitor; + impl<'de> serde::de::Visitor<'de> for FieldNameVisitor { + type Value = FieldName; + fn expecting(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("field identifier") + } + + #[inline] + fn visit_str(self, value: &str) -> Result + where + E: serde::de::Error, + { + match value { + $($field_key => Ok(FieldName::$field),)* + _ => Err(serde::de::Error::unknown_field(value, FIELDS)), + } + } + } + impl<'de> Deserialize<'de> for FieldName { + #[inline] + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + Deserializer::deserialize_identifier(deserializer, FieldNameVisitor) + } + } + struct Field; + impl<'de> serde::de::Visitor<'de> for Field { + type Value = $name; + fn expecting(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(concat!("struct ", stringify!($name))) + } + + #[inline] + fn visit_map(self, mut map: A) -> Result + where + A: serde::de::MapAccess<'de>, + { + $(let mut $field: Option<$field_ty> = None;)* + while let Some(key) = + match serde::de::MapAccess::next_key::(&mut map) { + Ok(val) => val, + Err(err) => { + return Err(err); + } + } + { + match key { + $(FieldName::$field => { + if $field.is_some() { + return Err(::duplicate_field( + $field_key, + )); + } + $field = match serde::de::MapAccess::next_value::<$field_ty>( + &mut map, + ) { + Ok(val) => Some(val), + Err(err) => { + return Err(err); + } + }; + })* + } + } + Ok($name { $($field),* }) + } + } + const FIELDS: &'static [&'static str] = &[ + $($field_key,)* + ]; + Deserializer::deserialize_struct( + deserializer, + stringify!($name), + FIELDS, + Field, + ) + } + } } } @@ -389,101 +480,101 @@ define_config! { /// TOML representation of various global build decisions. #[derive(Default)] struct Build { - build: Option, - host: Option>, - target: Option>, - build_dir: Option, - cargo: Option, - rustc: Option, - rustfmt: Option, - docs: Option, - compiler_docs: Option, - docs_minification: Option, - submodules: Option, - fast_submodules: Option, - gdb: Option, - nodejs: Option, - npm: Option, - python: Option, - locked_deps: Option, - vendor: Option, - full_bootstrap: Option, - extended: Option, - tools: Option>, - verbose: Option, - sanitizers: Option, - profiler: Option, - cargo_native_static: Option, - low_priority: Option, - configure_args: Option>, - local_rebuild: Option, - print_step_timings: Option, - print_step_rusage: Option, - check_stage: Option, - doc_stage: Option, - build_stage: Option, - test_stage: Option, - install_stage: Option, - dist_stage: Option, - bench_stage: Option, - patch_binaries_for_nix: Option, + build: Option = "build", + host: Option> = "host", + target: Option> = "target", + build_dir: Option = "build-dir", + cargo: Option = "cargo", + rustc: Option = "rustc", + rustfmt: Option = "rustfmt", + docs: Option = "docs", + compiler_docs: Option = "compiler-docs", + docs_minification: Option = "docs-minification", + submodules: Option = "submodules", + fast_submodules: Option = "fast-submodules", + gdb: Option = "gdb", + nodejs: Option = "nodejs", + npm: Option = "npm", + python: Option = "python", + locked_deps: Option = "locked-deps", + vendor: Option = "vendor", + full_bootstrap: Option = "full-bootstrap", + extended: Option = "extended", + tools: Option> = "tools", + verbose: Option = "verbose", + sanitizers: Option = "sanitizers", + profiler: Option = "profiler", + cargo_native_static: Option = "cargo-native-static", + low_priority: Option = "low-priority", + configure_args: Option> = "configure-args", + local_rebuild: Option = "local-rebuild", + print_step_timings: Option = "print-step-timings", + print_step_rusage: Option = "print-step-rusage", + check_stage: Option = "check-stage", + doc_stage: Option = "doc-stage", + build_stage: Option = "build-stage", + test_stage: Option = "test-stage", + install_stage: Option = "install-stage", + dist_stage: Option = "dist-stage", + bench_stage: Option = "bench-stage", + patch_binaries_for_nix: Option = "patch-binaries-for-nix", } } define_config! { /// TOML representation of various global install decisions. struct Install { - prefix: Option, - sysconfdir: Option, - docdir: Option, - bindir: Option, - libdir: Option, - mandir: Option, - datadir: Option, + prefix: Option = "prefix", + sysconfdir: Option = "sysconfdir", + docdir: Option = "docdir", + bindir: Option = "bindir", + libdir: Option = "libdir", + mandir: Option = "mandir", + datadir: Option = "datadir", } } define_config! { /// TOML representation of how the LLVM build is configured. struct Llvm { - skip_rebuild: Option, - optimize: Option, - thin_lto: Option, - release_debuginfo: Option, - assertions: Option, - tests: Option, - plugins: Option, - ccache: Option, - version_check: Option, - static_libstdcpp: Option, - ninja: Option, - targets: Option, - experimental_targets: Option, - link_jobs: Option, - link_shared: Option, - version_suffix: Option, - clang_cl: Option, - cflags: Option, - cxxflags: Option, - ldflags: Option, - use_libcxx: Option, - use_linker: Option, - allow_old_toolchain: Option, - polly: Option, - clang: Option, - download_ci_llvm: Option, - build_config: Option>, + skip_rebuild: Option = "skip-rebuild", + optimize: Option = "optimize", + thin_lto: Option = "thin-lto", + release_debuginfo: Option = "release-debuginfo", + assertions: Option = "assertions", + tests: Option = "tests", + plugins: Option = "plugins", + ccache: Option = "ccache", + version_check: Option = "version-check", + static_libstdcpp: Option = "static-libstdcpp", + ninja: Option = "ninja", + targets: Option = "targets", + experimental_targets: Option = "experimental-targets", + link_jobs: Option = "link-jobs", + link_shared: Option = "link-shared", + version_suffix: Option = "version-suffix", + clang_cl: Option = "clang-cl", + cflags: Option = "cflags", + cxxflags: Option = "cxxflags", + ldflags: Option = "ldflags", + use_libcxx: Option = "use-libcxx", + use_linker: Option = "use-linker", + allow_old_toolchain: Option = "allow-old-toolchain", + polly: Option = "polly", + clang: Option = "clang", + download_ci_llvm: Option = "download-ci-llvm", + build_config: Option> = "build-config", } } define_config! { struct Dist { - sign_folder: Option, - gpg_password_file: Option, - upload_addr: Option, - src_tarball: Option, - missing_tools: Option, - compression_formats: Option>, + sign_folder: Option = "sign-folder", + gpg_password_file: Option = "gpg-password-file", + upload_addr: Option = "upload-addr", + src_tarball: Option = "src-tarball", + missing_tools: Option = "missing-tools", + compression_formats: Option> = "compression-formats", } } @@ -503,76 +594,76 @@ impl Default for StringOrBool { define_config! { /// TOML representation of how the Rust build is configured. struct Rust { - optimize: Option, - debug: Option, - codegen_units: Option, - codegen_units_std: Option, - debug_assertions: Option, - debug_assertions_std: Option, - overflow_checks: Option, - overflow_checks_std: Option, - debug_logging: Option, - debuginfo_level: Option, - debuginfo_level_rustc: Option, - debuginfo_level_std: Option, - debuginfo_level_tools: Option, - debuginfo_level_tests: Option, - run_dsymutil: Option, - backtrace: Option, - incremental: Option, - parallel_compiler: Option, - default_linker: Option, - channel: Option, - description: Option, - musl_root: Option, - rpath: Option, - verbose_tests: Option, - optimize_tests: Option, - codegen_tests: Option, - ignore_git: Option, - dist_src: Option, - save_toolstates: Option, - codegen_backends: Option>, - lld: Option, - use_lld: Option, - llvm_tools: Option, - deny_warnings: Option, - backtrace_on_ice: Option, - verify_llvm_ir: Option, - thin_lto_import_instr_limit: Option, - remap_debuginfo: Option, - jemalloc: Option, - test_compare_mode: Option, - llvm_libunwind: Option, - control_flow_guard: Option, - new_symbol_mangling: Option, - profile_generate: Option, - profile_use: Option, + optimize: Option = "optimize", + debug: Option = "debug", + codegen_units: Option = "codegen-units", + codegen_units_std: Option = "codegen-units-std", + debug_assertions: Option = "debug-assertions", + debug_assertions_std: Option = "debug-assertions-std", + overflow_checks: Option = "overflow-checks", + overflow_checks_std: Option = "overflow-checks-std", + debug_logging: Option = "debug-logging", + debuginfo_level: Option = "debuginfo-level", + debuginfo_level_rustc: Option = "debuginfo-level-rustc", + debuginfo_level_std: Option = "debuginfo-level-std", + debuginfo_level_tools: Option = "debuginfo-level-tools", + debuginfo_level_tests: Option = "debuginfo-level-tests", + run_dsymutil: Option = "run-dsymutil", + backtrace: Option = "backtrace", + incremental: Option = "incremental", + parallel_compiler: Option = "parallel-compiler", + default_linker: Option = "default-linker", + channel: Option = "channel", + description: Option = "description", + musl_root: Option = "musl-root", + rpath: Option = "rpath", + verbose_tests: Option = "verbose-tests", + optimize_tests: Option = "optimize-tests", + codegen_tests: Option = "codegen-tests", + ignore_git: Option = "ignore-git", + dist_src: Option = "dist-src", + save_toolstates: Option = "save-toolstates", + codegen_backends: Option> = "codegen-backends", + lld: Option = "lld", + use_lld: Option = "use-lld", + llvm_tools: Option = "llvm-tools", + deny_warnings: Option = "deny-warnings", + backtrace_on_ice: Option = "backtrace-on-ice", + verify_llvm_ir: Option = "verify-llvm-ir", + thin_lto_import_instr_limit: Option = "thin-lto-import-instr-limit", + remap_debuginfo: Option = "remap-debuginfo", + jemalloc: Option = "jemalloc", + test_compare_mode: Option = "test-compare-mode", + llvm_libunwind: Option = "llvm-libunwind", + control_flow_guard: Option = "control-flow-guard", + new_symbol_mangling: Option = "new-symbol-mangling", + profile_generate: Option = "profile-generate", + profile_use: Option = "profile-use", // ignored; this is set from an env var set by bootstrap.py - download_rustc: Option, + download_rustc: Option = "download-rustc", } } define_config! { /// TOML representation of how each build target is configured. struct TomlTarget { - cc: Option, - cxx: Option, - ar: Option, - ranlib: Option, - default_linker: Option, - linker: Option, - llvm_config: Option, - llvm_filecheck: Option, - android_ndk: Option, - sanitizers: Option, - profiler: Option, - crt_static: Option, - musl_root: Option, - musl_libdir: Option, - wasi_root: Option, - qemu_rootfs: Option, - no_std: Option, + cc: Option = "cc", + cxx: Option = "cxx", + ar: Option = "ar", + ranlib: Option = "ranlib", + default_linker: Option = "default-linker", + linker: Option = "linker", + llvm_config: Option = "llvm-config", + llvm_filecheck: Option = "llvm-filecheck", + android_ndk: Option = "android-ndk", + sanitizers: Option = "sanitizers", + profiler: Option = "profiler", + crt_static: Option = "crt-static", + musl_root: Option = "musl-root", + musl_libdir: Option = "musl-libdir", + wasi_root: Option = "wasi-root", + qemu_rootfs: Option = "qemu-rootfs", + no_std: Option = "no-std", } } From dca8ff5b252ce2430fa61e6a5bd230b1edc9ea50 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Thu, 3 Mar 2022 18:44:02 +0100 Subject: [PATCH 3/4] Prevent duplicate monomorphization of deserialization impls This reduces binary size from 9.7MiB (5.8MiB for just rustbuild code) to 9.3MiB (5.3MiB for just rustbuild code). This doesn't reduce compile time in a statistically significant way. --- src/bootstrap/config.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/config.rs b/src/bootstrap/config.rs index 762f18fb32f8e..6e8471b1de7a3 100644 --- a/src/bootstrap/config.rs +++ b/src/bootstrap/config.rs @@ -731,7 +731,11 @@ impl Config { let contents = t!(fs::read_to_string(file), format!("config file {} not found", file.display())); - match toml::from_str(&contents) { + // Deserialize to Value and then TomlConfig to prevent the Deserialize impl of + // TomlConfig and sub types to be monomorphized 5x by toml. + match toml::from_str(&contents) + .and_then(|table: toml::Value| TomlConfig::deserialize(table)) + { Ok(table) => table, Err(err) => { println!("failed to parse TOML configuration '{}': {}", file.display(), err); From a0163f748719c743f0d53b66e41158fbd7dbd035 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Sun, 13 Mar 2022 17:35:02 +0100 Subject: [PATCH 4/4] Remove FieldName enum --- src/bootstrap/config.rs | 40 ++++++---------------------------------- 1 file changed, 6 insertions(+), 34 deletions(-) diff --git a/src/bootstrap/config.rs b/src/bootstrap/config.rs index 6e8471b1de7a3..e744810295def 100644 --- a/src/bootstrap/config.rs +++ b/src/bootstrap/config.rs @@ -389,37 +389,6 @@ macro_rules! define_config { where D: Deserializer<'de>, { - #[allow(non_camel_case_types)] - enum FieldName { - $($field,)* - } - struct FieldNameVisitor; - impl<'de> serde::de::Visitor<'de> for FieldNameVisitor { - type Value = FieldName; - fn expecting(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str("field identifier") - } - - #[inline] - fn visit_str(self, value: &str) -> Result - where - E: serde::de::Error, - { - match value { - $($field_key => Ok(FieldName::$field),)* - _ => Err(serde::de::Error::unknown_field(value, FIELDS)), - } - } - } - impl<'de> Deserialize<'de> for FieldName { - #[inline] - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - Deserializer::deserialize_identifier(deserializer, FieldNameVisitor) - } - } struct Field; impl<'de> serde::de::Visitor<'de> for Field { type Value = $name; @@ -434,15 +403,15 @@ macro_rules! define_config { { $(let mut $field: Option<$field_ty> = None;)* while let Some(key) = - match serde::de::MapAccess::next_key::(&mut map) { + match serde::de::MapAccess::next_key::(&mut map) { Ok(val) => val, Err(err) => { return Err(err); } } { - match key { - $(FieldName::$field => { + match &*key { + $($field_key => { if $field.is_some() { return Err(::duplicate_field( $field_key, @@ -457,6 +426,9 @@ macro_rules! define_config { } }; })* + key => { + return Err(serde::de::Error::unknown_field(key, FIELDS)); + } } } Ok($name { $($field),* })