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

Box ast::ItemKind #81400

Closed
wants to merge 1 commit into from
Closed

Box ast::ItemKind #81400

wants to merge 1 commit into from

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Jan 25, 2021

This reduces the size of ast::Item from 296 to 96 bytes.

See https://rust-lang.zulipchat.com/#narrow/stream/247081-t-compiler.2Fperformance/topic/windows-rs.20perf/near/223974946 for context; the hope is this will save memory compiling windows-rs.

This reduces the size of ast::Item from 296 to 96 bytes.
@jyn514 jyn514 added A-parser Area: The parsing of Rust source code to an AST T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. I-compilemem Issue: Problems and improvements with respect to memory usage during compilation. labels Jan 25, 2021
@rust-highfive
Copy link
Collaborator

r? @davidtwco

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 25, 2021
@jyn514
Copy link
Member Author

jyn514 commented Jan 25, 2021

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@bors
Copy link
Contributor

bors commented Jan 25, 2021

⌛ Trying commit 6f5d4b8 with merge 76f647b0d68bb484dfaf12d4025354e07fdda7c6...

@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
configure: rust.channel         := nightly
configure: rust.debug-assertions := True
configure: llvm.assertions      := True
configure: dist.missing-tools   := True
configure: build.configure-args := ['--enable-sccache', '--disable-manage-submodu ...
configure: writing `config.toml` in current directory
configure: 
configure: run `python /checkout/x.py --help`
configure: 
---
    Checking rustc_parse_format v0.0.0 (/checkout/compiler/rustc_parse_format)
error[E0308]: mismatched types
    --> /checkout/compiler/rustc_data_structures/src/macros.rs:5:32
     |
3    | / macro_rules! static_assert_size {
4    | |     ($ty:ty, $size:expr) => {
5    | |         const _: [(); $size] = [(); ::std::mem::size_of::<$ty>()];
     | |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected an array with a fixed size of 96 elements, found one with 68 elements
7    | | }
     | |_- in this expansion of `rustc_data_structures::static_assert_size!`
     | 
    ::: compiler/rustc_ast/src/ast.rs:2596:1
    ::: compiler/rustc_ast/src/ast.rs:2596:1
     |
2596 |   rustc_data_structures::static_assert_size!(Item<ItemKind>, 96);
     |   --------------------------------------------------------------- in this macro invocation
error[E0308]: mismatched types
    --> /checkout/compiler/rustc_data_structures/src/macros.rs:5:32
     |
3    | / macro_rules! static_assert_size {
3    | / macro_rules! static_assert_size {
4    | |     ($ty:ty, $size:expr) => {
5    | |         const _: [(); $size] = [(); ::std::mem::size_of::<$ty>()];
     | |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected an array with a fixed size of 96 elements, found one with 68 elements
7    | | }
     | |_- in this expansion of `rustc_data_structures::static_assert_size!`
     | 
    ::: compiler/rustc_ast/src/ast.rs:2597:1
    ::: compiler/rustc_ast/src/ast.rs:2597:1
     |
2597 |   rustc_data_structures::static_assert_size!(Item<AssocItemKind>, 96);
     |   -------------------------------------------------------------------- in this macro invocation
error[E0308]: mismatched types
    --> /checkout/compiler/rustc_data_structures/src/macros.rs:5:32
     |
3    | / macro_rules! static_assert_size {
3    | / macro_rules! static_assert_size {
4    | |     ($ty:ty, $size:expr) => {
5    | |         const _: [(); $size] = [(); ::std::mem::size_of::<$ty>()];
     | |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected an array with a fixed size of 96 elements, found one with 68 elements
7    | | }
     | |_- in this expansion of `rustc_data_structures::static_assert_size!`
     | 
    ::: compiler/rustc_ast/src/ast.rs:2598:1
    ::: compiler/rustc_ast/src/ast.rs:2598:1
     |
2598 |   rustc_data_structures::static_assert_size!(Item<ForeignItemKind>, 96);
     |   ---------------------------------------------------------------------- in this macro invocation
error[E0308]: mismatched types
    --> /checkout/compiler/rustc_data_structures/src/macros.rs:5:32
     |
3    | / macro_rules! static_assert_size {
3    | / macro_rules! static_assert_size {
4    | |     ($ty:ty, $size:expr) => {
5    | |         const _: [(); $size] = [(); ::std::mem::size_of::<$ty>()];
     | |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected an array with a fixed size of 96 elements, found one with 68 elements
7    | | }
     | |_- in this expansion of `rustc_data_structures::static_assert_size!`
     | 
    ::: compiler/rustc_ast/src/ast.rs:2596:1
    ::: compiler/rustc_ast/src/ast.rs:2596:1
     |
2596 |   rustc_data_structures::static_assert_size!(Item<ItemKind>, 96);
     |   --------------------------------------------------------------- in this macro invocation
error[E0308]: mismatched types
    --> /checkout/compiler/rustc_data_structures/src/macros.rs:5:32
     |
3    | / macro_rules! static_assert_size {
3    | / macro_rules! static_assert_size {
4    | |     ($ty:ty, $size:expr) => {
5    | |         const _: [(); $size] = [(); ::std::mem::size_of::<$ty>()];
     | |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected an array with a fixed size of 96 elements, found one with 68 elements
7    | | }
     | |_- in this expansion of `rustc_data_structures::static_assert_size!`
     | 
    ::: compiler/rustc_ast/src/ast.rs:2597:1
    ::: compiler/rustc_ast/src/ast.rs:2597:1
     |
2597 |   rustc_data_structures::static_assert_size!(Item<AssocItemKind>, 96);
     |   -------------------------------------------------------------------- in this macro invocation
error[E0308]: mismatched types
    --> /checkout/compiler/rustc_data_structures/src/macros.rs:5:32
     |
3    | / macro_rules! static_assert_size {
3    | / macro_rules! static_assert_size {
4    | |     ($ty:ty, $size:expr) => {
5    | |         const _: [(); $size] = [(); ::std::mem::size_of::<$ty>()];
     | |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected an array with a fixed size of 96 elements, found one with 68 elements
7    | | }
     | |_- in this expansion of `rustc_data_structures::static_assert_size!`
     | 
    ::: compiler/rustc_ast/src/ast.rs:2598:1
    ::: compiler/rustc_ast/src/ast.rs:2598:1
     |
2598 |   rustc_data_structures::static_assert_size!(Item<ForeignItemKind>, 96);
     |   ---------------------------------------------------------------------- in this macro invocation
    Checking tracing-serde v0.1.2
    Checking gsgdt v0.1.2
    Checking rls-span v0.5.3
    Checking rls-data v0.19.1
---
error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0308`.
error: build failed
command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "check" "--target" "i686-pc-windows-gnu" "-Zbinary-dep-depinfo" "-j" "16" "--release" "--locked" "--color" "always" "--features" " llvm" "--manifest-path" "/checkout/compiler/rustc/Cargo.toml" "--all-targets" "-p" "rustc-main" "-p" "rustc_driver" "-p" "rustc_mir" "-p" "rustc_graphviz" "-p" "rustc_index" "-p" "rustc_trait_selection" "-p" "rustc_parse_format" "-p" "rustc_attr" "-p" "rustc_infer" "-p" "coverage_test_macros" "-p" "rustc_macros" "-p" "rustc_apfloat" "-p" "rustc_lexer" "-p" "rustc_lint" "-p" "rustc_error_codes" "-p" "rustc_plugin_impl" "-p" "rustc_interface" "-p" "rustc_passes" "-p" "rustc_resolve" "-p" "rustc_arena" "-p" "rustc_ast_passes" "-p" "rustc_privacy" "-p" "rustc_incremental" "-p" "rustc_fs_util" "-p" "rustc_symbol_mangling" "-p" "rustc_expand" "-p" "rustc_builtin_macros" "-p" "rustc_typeck" "-p" "rustc_ast_lowering" "-p" "rustc_traits" "-p" "rustc_mir_build" "-p" "rustc_codegen_llvm" "-p" "rustc_llvm" "-p" "rustc_ty_utils" "-p" "rustc_save_analysis" "-p" "rustc_session" "-p" "rustc_lint_defs" "-p" "rustc_parse" "-p" "rustc_errors" "-p" "rustc_serialize" "-p" "rustc_hir_pretty" "-p" "rustc_feature" "-p" "rustc_hir" "-p" "rustc_middle" "-p" "rustc_type_ir" "-p" "rustc_query_system" "-p" "rustc_data_structures" "-p" "rustc_ast" "-p" "rustc_target" "-p" "rustc_ast_pretty" "-p" "rustc_metadata" "-p" "rustc_span" "-p" "rustc_codegen_ssa" "--message-format" "json-render-diagnostics"
failed to run: /checkout/obj/build/bootstrap/debug/bootstrap check --target=i686-pc-windows-gnu --host=i686-pc-windows-gnu --all-targets
Build completed unsuccessfully in 0:01:16

@bors
Copy link
Contributor

bors commented Jan 26, 2021

☀️ Try build successful - checks-actions
Build commit: 76f647b0d68bb484dfaf12d4025354e07fdda7c6 (76f647b0d68bb484dfaf12d4025354e07fdda7c6)

@rust-timer
Copy link
Collaborator

Queued 76f647b0d68bb484dfaf12d4025354e07fdda7c6 with parent f4eb5d9, future comparison URL.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 26, 2021
@ThePuzzlemaker
Copy link
Contributor

ThePuzzlemaker commented Jan 26, 2021

The fact that this had to be done genuinely scares me.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (76f647b0d68bb484dfaf12d4025354e07fdda7c6): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 26, 2021
@@ -2593,6 +2593,10 @@ pub struct Item<K = ItemKind> {
pub tokens: Option<LazyTokenStream>,
}

rustc_data_structures::static_assert_size!(Item<ItemKind>, 96);
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this account for pointer size differences across platforms?

@bugadani
Copy link
Contributor

Well this has the exact opposite effect: increasing memory use. On the flip side, a small perf gain.

@bugadani
Copy link
Contributor

bugadani commented Jan 26, 2021

The biggest ItemKind variants are:

  • Impl: 200 bytes
  • Fn: 176 bytes
  • Trait: 136 bytes
  • TyAlias: 120 bytes

@davidtwco
Copy link
Member

Should this be closed in favour of #81405 or are there more things you'd like to try here @jyn514?

@davidtwco davidtwco added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 30, 2021
@jyn514
Copy link
Member Author

jyn514 commented Jan 30, 2021

I'm not planning to follow up on this.

@jyn514 jyn514 closed this Jan 30, 2021
@jyn514 jyn514 deleted the box-ast-kind branch January 30, 2021 14:40
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 2, 2021
Box the biggest ast::ItemKind variants

This PR is a different approach on rust-lang#81400, aiming to save memory in humongous ASTs.

The three affected item kind enums are:
 - `ast::ItemKind` (208 -> 112 bytes)
 - `ast::AssocItemKind` (176 -> 72 bytes)
 - `ast::ForeignItemKind` (176 -> 72 bytes)
Manishearth pushed a commit to Manishearth/rust-clippy that referenced this pull request Feb 3, 2021
Box the biggest ast::ItemKind variants

This PR is a different approach on rust-lang/rust#81400, aiming to save memory in humongous ASTs.

The three affected item kind enums are:
 - `ast::ItemKind` (208 -> 112 bytes)
 - `ast::AssocItemKind` (176 -> 72 bytes)
 - `ast::ForeignItemKind` (176 -> 72 bytes)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-parser Area: The parsing of Rust source code to an AST I-compilemem Issue: Problems and improvements with respect to memory usage during compilation. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants