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

De-querify trivial_dropck_outlives. #66012

Merged
merged 1 commit into from
Nov 5, 2019

Conversation

nnethercote
Copy link
Contributor

It's sufficiently simple and fast that memoizing it is a slight
pessimization.

r? @michaelwoerister

It's sufficiently simple and fast that memoizing it is a slight
pessimization.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 1, 2019
@nnethercote
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Nov 1, 2019

⌛ Trying commit 8971972 with merge 06e3c5c6c65f3bcce1f70f0b161172fe840ef740...

@nnethercote
Copy link
Contributor Author

I'm not certain this is worthwhile, but I've been looking at the parallel compiler running with one thread and get_query() accounts for the vast majority of the additional overhead. Anything that can be cheaply computed on-demand instead of going via a query (which involves a shard lookup, locking, and a hashmap lookup, and contention for all of this) seems good.

The self-profile stats on perf.rust-lang.org show that trivial_dropck_outlives is one of the more commonly-called queries, assuming I'm reading the stats correctly; if you sort by "Invocations" it's usually at or near the top.

@bors
Copy link
Contributor

bors commented Nov 1, 2019

☀️ Try build successful - checks-azure
Build commit: 06e3c5c6c65f3bcce1f70f0b161172fe840ef740 (06e3c5c6c65f3bcce1f70f0b161172fe840ef740)

@rust-timer
Copy link
Collaborator

Queued 06e3c5c6c65f3bcce1f70f0b161172fe840ef740 with parent aa4e57c, future comparison URL.

@michaelwoerister
Copy link
Member

Thanks, @nnethercote! r=me if it doesn't regress performance.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit 06e3c5c6c65f3bcce1f70f0b161172fe840ef740, comparison URL.

@nnethercote
Copy link
Contributor Author

It's a small instruction counts win, up to 1%.

@bors r=michaelwoerister

@bors
Copy link
Contributor

bors commented Nov 1, 2019

📌 Commit 8971972 has been approved by michaelwoerister

@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 Nov 1, 2019
tmandry added a commit to tmandry/rust that referenced this pull request Nov 1, 2019
…outlives, r=michaelwoerister

De-querify `trivial_dropck_outlives`.

It's sufficiently simple and fast that memoizing it is a slight
pessimization.

r? @michaelwoerister
tmandry added a commit to tmandry/rust that referenced this pull request Nov 1, 2019
…outlives, r=michaelwoerister

De-querify `trivial_dropck_outlives`.

It's sufficiently simple and fast that memoizing it is a slight
pessimization.

r? @michaelwoerister
@@ -208,15 +207,15 @@ pub fn trivial_dropck_outlives<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> bool {
| ty::Error => true,

// [T; N] and [T] have same properties as T.
ty::Array(ty, _) | ty::Slice(ty) => tcx.trivial_dropck_outlives(ty),
ty::Array(ty, _) | ty::Slice(ty) => trivial_dropck_outlives(tcx, ty),
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add some comment on why this can't result in infinite recursion? Since the query system isn't around to prevent that. This function could perhaps be a method on TyCtxt to avoid changing use sites.

pietroalbini added a commit to pietroalbini/rust that referenced this pull request Nov 5, 2019
…outlives, r=michaelwoerister

De-querify `trivial_dropck_outlives`.

It's sufficiently simple and fast that memoizing it is a slight
pessimization.

r? @michaelwoerister
bors added a commit that referenced this pull request Nov 5, 2019
Rollup of 8 pull requests

Successful merges:

 - #65948 (Improve MaybeUninit::get_{ref,mut} documentation)
 - #65953 (Allow specifying LLVM's MCTargetOptions::ABIName in target specification files)
 - #66012 (De-querify `trivial_dropck_outlives`.)
 - #66025 (`Span` cannot represent `span.hi < span.lo`)
 - #66047 (Don't double-count `simd_shuffle` promotion candidates)
 - #66053 (when Miri tests are not passing, do not add Miri component)
 - #66082 (clean highlightSourceLines code)
 - #66091 (Implemented the home_dir for VxWorks)

Failed merges:

r? @ghost
@bors bors merged commit 8971972 into rust-lang:master Nov 5, 2019
@nnethercote nnethercote deleted the dequery-trivial_dropck_outlives branch November 6, 2019 04:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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