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 ItemKind::Impl #80961

Closed
wants to merge 2 commits into from
Closed

Box ItemKind::Impl #80961

wants to merge 2 commits into from

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Jan 13, 2021

This brings the size of ItemKind down from 128 to 88 bytes.
Hopefully this will have perf improvements.

Follow-up to #79322 (comment), cc @estebank

@jyn514 jyn514 added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-HIR Area: The high-level intermediate representation (HIR) I-compilemem Issue: Problems and improvements with respect to memory usage during compilation. S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jan 13, 2021
@rust-highfive
Copy link
Collaborator

r? @varkor

(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 13, 2021
@jyn514
Copy link
Member Author

jyn514 commented Jan 13, 2021

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@bors
Copy link
Contributor

bors commented Jan 13, 2021

⌛ Trying commit 8bd6427632c222fc019a624a88edcac1efa9c1fe with merge 764bf6c03d3cac0149069e4345da9fcaed5bc45d...

@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_errors v0.0.0 (/checkout/compiler/rustc_errors)
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 88 elements, found one with 60 elements
7    | | }
     | |_- in this expansion of `static_assert_size!`
     | 
     | 
    ::: compiler/rustc_hir/src/hir.rs:2568:1
     |
2568 |   static_assert_size!(ItemKind<'_>, 88);
     |   -------------------------------------- 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 88 elements, found one with 60 elements
7    | | }
     | |_- in this expansion of `static_assert_size!`
     | 
     | 
    ::: compiler/rustc_hir/src/hir.rs:2568:1
     |
2568 |   static_assert_size!(ItemKind<'_>, 88);
     |   -------------------------------------- in this macro invocation
    Checking rustc_session v0.0.0 (/checkout/compiler/rustc_session)
    Checking rustc_query_system v0.0.0 (/checkout/compiler/rustc_query_system)
error: aborting due to previous error

---

To learn more, run the command again with --verbose.
warning: build failed, waiting for other jobs to finish...
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_errors" "-p" "rustc_macros" "-p" "rustc_lint_defs" "-p" "rustc_data_structures" "-p" "rustc_index" "-p" "rustc_graphviz" "-p" "rustc_serialize" "-p" "rustc_save_analysis" "-p" "rustc_lexer" "-p" "rustc_mir" "-p" "rustc_attr" "-p" "rustc_infer" "-p" "rustc_trait_selection" "-p" "rustc_parse_format" "-p" "rustc_apfloat" "-p" "coverage_test_macros" "-p" "rustc_plugin_impl" "-p" "rustc_hir_pretty" "-p" "rustc_ast_pretty" "-p" "rustc_span" "-p" "rustc_arena" "-p" "rustc_target" "-p" "rustc_metadata" "-p" "rustc_expand" "-p" "rustc_ast_passes" "-p" "rustc_parse" "-p" "rustc_ast" "-p" "rustc_session" "-p" "rustc_fs_util" "-p" "rustc_lint" "-p" "rustc_interface" "-p" "rustc_passes" "-p" "rustc_typeck" "-p" "rustc_ty_utils" "-p" "rustc_privacy" "-p" "rustc_traits" "-p" "rustc_codegen_llvm" "-p" "rustc_llvm" "-p" "rustc_symbol_mangling" "-p" "rustc_mir_build" "-p" "rustc_builtin_macros" "-p" "rustc_incremental" "-p" "rustc_ast_lowering" "-p" "rustc_resolve" "-p" "rustc_middle" "-p" "rustc_type_ir" "-p" "rustc_query_system" "-p" "rustc_feature" "-p" "rustc_hir" "-p" "rustc_error_codes" "-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:02

This makes it possible to pass the `Impl` directly to functions, instead
of having to pass each of the many fields one at a time. It also
simplifies matches in many cases.
This brings the size of ItemKind down from 128 to 88 bytes.
Hopefully this will have perf improvements.
@jyn514
Copy link
Member Author

jyn514 commented Jan 13, 2021

@rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@jyn514
Copy link
Member Author

jyn514 commented Jan 13, 2021

@rust-timer build 764bf6c03d3cac0149069e4345da9fcaed5bc45d

@rust-timer
Copy link
Collaborator

Queued 764bf6c03d3cac0149069e4345da9fcaed5bc45d with parent 058a710, future comparison URL.

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

@bors
Copy link
Contributor

bors commented Jan 13, 2021

☔ The latest upstream changes (presumably #79322) made this pull request unmergeable. Please resolve the merge conflicts.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (764bf6c03d3cac0149069e4345da9fcaed5bc45d): 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 13, 2021
@jyn514
Copy link
Member Author

jyn514 commented Jan 13, 2021

Those numbers are not very promising :(

@estebank
Copy link
Contributor

@estebank
Copy link
Contributor

I guess that going through the heap just makes things slower, it's fine. It's good to try things out and verify :)

@estebank estebank closed this Jan 13, 2021
@jyn514 jyn514 deleted the box-impl branch January 13, 2021 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-HIR Area: The high-level intermediate representation (HIR) I-compilemem Issue: Problems and improvements with respect to memory usage during compilation. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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.

8 participants