Skip to content

Deduplicate types in the generator witness #68672

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Feb 2, 2020
Merged

Deduplicate types in the generator witness #68672

merged 1 commit into from
Feb 2, 2020

Conversation

jonas-schievink
Copy link
Contributor

@jonas-schievink jonas-schievink commented Jan 30, 2020

For the await-call-tree benchmark this often reduces the types inside the witness from 12 to 2.

@rust-highfive
Copy link
Contributor

r? @petrochenkov

(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 30, 2020
@jonas-schievink
Copy link
Contributor Author

r? @Zoxc

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@rust-highfive rust-highfive assigned Zoxc and unassigned petrochenkov Jan 30, 2020
@bors
Copy link
Collaborator

bors commented Jan 30, 2020

⌛ Trying commit da3a4f503c0651b12250fb3b71113e79abe4d9b4 with merge 2ec126008c2c57c4cc1bc054e44051fea4a1e610...

@rust-highfive

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Jan 30, 2020

☀️ Try build successful - checks-azure
Build commit: 2ec126008c2c57c4cc1bc054e44051fea4a1e610 (2ec126008c2c57c4cc1bc054e44051fea4a1e610)

@rust-timer
Copy link
Collaborator

Queued 2ec126008c2c57c4cc1bc054e44051fea4a1e610 with parent c4071d0, future comparison URL.

@Zoxc
Copy link
Contributor

Zoxc commented Jan 30, 2020

I don't think this is the approach we should use to deduplicate the types, but it should be good enough for a perf test.

We'd want to erase regions before deduplicating types (otherwise &'a () and &'b () won't get deduplicated) and then let generator_interior_types be a map from an erased type to a list of causes.

@Zoxc
Copy link
Contributor

Zoxc commented Jan 30, 2020

On a second thought, maybe a list of causes would be too noisy for error messages, so maybe just sticking to one cause is fine.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit 2ec126008c2c57c4cc1bc054e44051fea4a1e610, comparison URL.

@Zoxc
Copy link
Contributor

Zoxc commented Jan 30, 2020

Yeah, this helped a bit too.

@jonas-schievink
Copy link
Contributor Author

@Zoxc Okay so it looks like storing a HashMap/Set keyed off of Tys inside the TypeckTables doesn't work due to the missing StableHash impl (which makes sense). I've used an FxHashSet during construction of the generator_interior_types instead.

@jonas-schievink jonas-schievink changed the title [WIP] [perf] Deduplicate types in the generator witness Deduplicate types in the generator witness Jan 31, 2020
@jonas-schievink
Copy link
Contributor Author

jonas-schievink commented Jan 31, 2020

(It also looks like the order is important for diagnostics, so using a HashMap/Set might cause incorrect diagnostics or nondeterminism there)

@jonas-schievink
Copy link
Contributor Author

@bors rollup=never

@Zoxc
Copy link
Contributor

Zoxc commented Feb 1, 2020

@bors r+

@bors
Copy link
Collaborator

bors commented Feb 1, 2020

📌 Commit 791123d has been approved by Zoxc

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 1, 2020
@bors
Copy link
Collaborator

bors commented Feb 2, 2020

⌛ Testing commit 791123d with merge bc4a339...

bors added a commit that referenced this pull request Feb 2, 2020
Deduplicate types in the generator witness

For the `await-call-tree` benchmark this often reduces the types inside the witness from 12 to 2.
@bors
Copy link
Collaborator

bors commented Feb 2, 2020

☀️ Test successful - checks-azure
Approved by: Zoxc
Pushing bc4a339 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 2, 2020
@bors bors merged commit 791123d into rust-lang:master Feb 2, 2020
@Zoxc Zoxc mentioned this pull request Feb 2, 2020
@jonas-schievink jonas-schievink deleted the dedup-witness branch February 3, 2020 10:42
bors added a commit that referenced this pull request Feb 3, 2020
Re-land "add IntoFuture trait and support for await"

Testing the code from #65244 to see if the performance regressions are still there. #68606 and #68672 made perf optimizations that might interact with this change.

If this lands, fixes #67982.

cc @seanmonstar @jonas-schievink
r? @cramertj
@jonas-schievink jonas-schievink added relnotes Marks issues that should be documented in the release notes of the next release. relnotes-perf Performance improvements that should be mentioned in the release notes. labels Mar 13, 2020
@jonas-schievink jonas-schievink modified the milestones: 1.44, 1.43 Mar 13, 2020
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request May 16, 2020
Pkgsrc changes:
 * Bump rust bootstrap version to 1.42.0, except for Darwin/i686 where the
   bootstrap is not (yet?) available.

Upstream changes:

Version 1.43.0 (2020-04-23)
==========================

Language
--------
- [Fixed using binary operations with `&{number}` (e.g. `&1.0`) not having
  the type inferred correctly.][68129]
- [Attributes such as `#[cfg()]` can now be used on `if` expressions.][69201]

**Syntax only changes**
- [Allow `type Foo: Ord` syntactically.][69361]
- [Fuse associated and extern items up to defaultness.][69194]
- [Syntactically allow `self` in all `fn` contexts.][68764]
- [Merge `fn` syntax + cleanup item parsing.][68728]
- [`item` macro fragments can be interpolated into `trait`s, `impl`s,
  and `extern` blocks.][69366]
  For example, you may now write:
  ```rust
  macro_rules! mac_trait {
      ($i:item) => {
          trait T { $i }
      }
  }
  mac_trait! {
      fn foo() {}
  }
  ```
These are still rejected *semantically*, so you will likely receive an error but
these changes can be seen and parsed by macros and
conditional compilation.


Compiler
--------
- [You can now pass multiple lint flags to rustc to override the previous
  flags.][67885] For example; `rustc -D unused -A unused-variables` denies
  everything in the `unused` lint group except `unused-variables` which
  is explicitly allowed. However, passing `rustc -A unused-variables -D unused` denies
  everything in the `unused` lint group **including** `unused-variables` since
  the allow flag is specified before the deny flag (and therefore overridden).
- [rustc will now prefer your system MinGW libraries over its bundled libraries
  if they are available on `windows-gnu`.][67429]
- [rustc now buffers errors/warnings printed in JSON.][69227]

Libraries
---------
- [`Arc<[T; N]>`, `Box<[T; N]>`, and `Rc<[T; N]>`, now implement
  `TryFrom<Arc<[T]>>`,`TryFrom<Box<[T]>>`, and `TryFrom<Rc<[T]>>`
  respectively.][69538] **Note** These conversions are only available when `N`
  is `0..=32`.
- [You can now use associated constants on floats and integers directly, rather
  than having to import the module.][68952] e.g. You can now write `u32::MAX` or
  `f32::NAN` with no imports.
- [`u8::is_ascii` is now `const`.][68984]
- [`String` now implements `AsMut<str>`.][68742]
- [Added the `primitive` module to `std` and `core`.][67637] This module
  reexports Rust's primitive types. This is mainly useful in macros
  where you want avoid these types being shadowed.
- [Relaxed some of the trait bounds on `HashMap` and `HashSet`.][67642]
- [`string::FromUtf8Error` now implements `Clone + Eq`.][68738]

Stabilized APIs
---------------
- [`Once::is_completed`]
- [`f32::LOG10_2`]
- [`f32::LOG2_10`]
- [`f64::LOG10_2`]
- [`f64::LOG2_10`]
- [`iter::once_with`]

Cargo
-----
- [You can now set config `[profile]`s in your `.cargo/config`, or through
  your environment.][cargo/7823]
- [Cargo will now set `CARGO_BIN_EXE_<name>` pointing to a binary's
  executable path when running integration tests or benchmarks.][cargo/7697]
  `<name>` is the name of your binary as-is e.g. If you wanted the executable
  path for a binary named `my-program`you would use
  `env!("CARGO_BIN_EXE_my-program")`.

Misc
----
- [Certain checks in the `const_err` lint were deemed unrelated to const
  evaluation][69185], and have been moved to the `unconditional_panic` and
  `arithmetic_overflow` lints.

Compatibility Notes
-------------------

- [Having trailing syntax in the `assert!` macro is now a hard error.][69548]
  This has been a warning since 1.36.0.
- [Fixed `Self` not having the correctly inferred type.][69340] This incorrectly
  led to some instances being accepted, and now correctly emits a hard error.

[69340]: rust-lang/rust#69340

Internal Only
-------------
These changes provide no direct user facing benefits, but represent significant
improvements to the internals and overall performance of `rustc` and
related tools.

- [All components are now built with `opt-level=3` instead of `2`.][67878]
- [Improved how rustc generates drop code.][67332]
- [Improved performance from `#[inline]`-ing certain hot functions.][69256]
- [traits: preallocate 2 Vecs of known initial size][69022]
- [Avoid exponential behaviour when relating types][68772]
- [Skip `Drop` terminators for enum variants without drop glue][68943]
- [Improve performance of coherence checks][68966]
- [Deduplicate types in the generator witness][68672]
- [Invert control in struct_lint_level.][68725]

[67332]: rust-lang/rust#67332
[67429]: rust-lang/rust#67429
[67637]: rust-lang/rust#67637
[67642]: rust-lang/rust#67642
[67878]: rust-lang/rust#67878
[67885]: rust-lang/rust#67885
[68129]: rust-lang/rust#68129
[68672]: rust-lang/rust#68672
[68725]: rust-lang/rust#68725
[68728]: rust-lang/rust#68728
[68738]: rust-lang/rust#68738
[68742]: rust-lang/rust#68742
[68764]: rust-lang/rust#68764
[68772]: rust-lang/rust#68772
[68943]: rust-lang/rust#68943
[68952]: rust-lang/rust#68952
[68966]: rust-lang/rust#68966
[68984]: rust-lang/rust#68984
[69022]: rust-lang/rust#69022
[69185]: rust-lang/rust#69185
[69194]: rust-lang/rust#69194
[69201]: rust-lang/rust#69201
[69227]: rust-lang/rust#69227
[69548]: rust-lang/rust#69548
[69256]: rust-lang/rust#69256
[69361]: rust-lang/rust#69361
[69366]: rust-lang/rust#69366
[69538]: rust-lang/rust#69538
[cargo/7823]: rust-lang/cargo#7823
[cargo/7697]: rust-lang/cargo#7697
[`Once::is_completed`]: https://doc.rust-lang.org/std/sync/struct.Once.html#method.is_completed
[`f32::LOG10_2`]: https://doc.rust-lang.org/std/f32/consts/constant.LOG10_2.html
[`f32::LOG2_10`]: https://doc.rust-lang.org/std/f32/consts/constant.LOG2_10.html
[`f64::LOG10_2`]: https://doc.rust-lang.org/std/f64/consts/constant.LOG10_2.html
[`f64::LOG2_10`]: https://doc.rust-lang.org/std/f64/consts/constant.LOG2_10.html
[`iter::once_with`]: https://doc.rust-lang.org/std/iter/fn.once_with.html
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
merged-by-bors This PR was explicitly merged by bors. relnotes Marks issues that should be documented in the release notes of the next release. relnotes-perf Performance improvements that should be mentioned in the release notes. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants