Construct query job latches on-demand #68693

Zoxc · 2020-01-31T06:22:01Z

r? @michaelwoerister

Zoxc · 2020-01-31T06:22:26Z

@bors try

bors · 2020-01-31T06:22:37Z

⌛ Trying commit aa0abc80119e8bb3b5f2f104b17b19a289a95121 with merge 38be9357b39a77cd206eab30844c05a019dc72f3...

Zoxc · 2020-01-31T07:11:26Z

@bors try @rust-timer queue

rust-timer · 2020-01-31T07:11:27Z

Awaiting bors try build completion

bors · 2020-01-31T07:11:36Z

⌛ Trying commit 8ecd740 with merge 7ccaf1c...

@michaelwoerister

[WIP] Construct query job latches on-demand r? @michaelwoerister

rust-highfive · 2020-01-31T08:59:53Z

The job dist-x86_64-linux-alt of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

2020-01-31T07:08:22.4499390Z    Compiling fmt_macros v0.0.0 (/checkout/src/libfmt_macros)
2020-01-31T07:08:24.3231932Z    Compiling rustc_session v0.0.0 (/checkout/src/librustc_session)
2020-01-31T07:08:46.3196047Z    Compiling rustc_hir v0.0.0 (/checkout/src/librustc_hir)
2020-01-31T07:09:01.1971460Z    Compiling rustc_parse v0.0.0 (/checkout/src/librustc_parse)
2020-01-31T07:09:16.9765290Z error[E0412]: cannot find type `PhantomData` in this scope
2020-01-31T07:09:16.9766448Z   --> src/librustc/ty/query/job.rs:85:12
2020-01-31T07:09:16.9766929Z    |
2020-01-31T07:09:16.9767497Z 85 |     dummy: PhantomData<QueryLatch<'tcx>>,
2020-01-31T07:09:16.9769089Z    |
2020-01-31T07:09:16.9769703Z help: possible candidates are found in other modules, you can import them into scope
2020-01-31T07:09:16.9770184Z    |
2020-01-31T07:09:16.9770696Z 1  | use core::marker::PhantomData;
2020-01-31T07:09:16.9770696Z 1  | use core::marker::PhantomData;
2020-01-31T07:09:16.9771153Z    |
2020-01-31T07:09:16.9771937Z 1  | use std::marker::PhantomData;
2020-01-31T07:09:16.9772418Z    |
2020-01-31T07:09:16.9775936Z 
2020-01-31T07:09:16.9987490Z error[E0425]: cannot find value `PhantomData` in this scope
2020-01-31T07:09:16.9989232Z   --> src/librustc/ty/query/job.rs:97:20
2020-01-31T07:09:16.9989746Z    |
2020-01-31T07:09:16.9990300Z 97 |             dummy: PhantomData,
2020-01-31T07:09:16.9991412Z    |
2020-01-31T07:09:16.9991989Z help: possible candidates are found in other modules, you can import them into scope
2020-01-31T07:09:16.9992443Z    |
2020-01-31T07:09:16.9992984Z 1  | use core::marker::PhantomData;
---
2020-01-31T07:09:46.2145077Z   local time: Fri Jan 31 07:09:46 UTC 2020
2020-01-31T07:09:46.9226763Z   network time: Fri, 31 Jan 2020 07:09:46 GMT
2020-01-31T07:09:46.9227114Z == end clock drift check ==
2020-01-31T07:09:47.2857182Z 
2020-01-31T07:09:47.2965292Z ##[error]Bash exited with code '1'.
2020-01-31T07:09:47.3038499Z ##[section]Starting: Checkout rust-lang/rust@try to s
2020-01-31T07:09:47.3040808Z ==============================================================================
2020-01-31T07:09:47.3040923Z Task         : Get sources
2020-01-31T07:09:47.3041019Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

bors · 2020-01-31T09:49:17Z

☀️ Try build successful - checks-azure
Build commit: 7ccaf1c (7ccaf1cdfde8cd513f14604879722adc5fb6dee8)

rust-timer · 2020-01-31T09:49:19Z

Queued 7ccaf1c with parent 266ecd6, future comparison URL.

rust-timer · 2020-01-31T14:18:49Z

Finished benchmarking try commit 7ccaf1c, comparison URL.

Zoxc · 2020-01-31T21:32:04Z

The idea here is to avoid creating a Lrc<QueryJob> each time we want to execute a query and instead create a QueryLatch = Lrc<Mutex<QueryLatchInfo<'tcx>>> when we actually want to wait for a query.

A QueryJob is now stored inline in the active map and contains a Option<QueryLatch> which is created on demand. This way we avoid the allocation and the atomic operations when there's no waiters on the query.

We can no longer refer to queries using Lrc<QueryJob> which is a small type. Using Query<'tcx> is too expensive as that is quite big. Instead QueryToken is introduced which is a pointer to a stack value which uniquely identifies an active query execution. A QueryToken is stored for each running query in the active map. This allows you to look up information for a token by looking at all the active maps. While this is expensive, it only happens on cycle errors and ICEs.

Zoxc · 2020-01-31T23:34:15Z

@bors try @rust-timer queue

rust-timer · 2020-01-31T23:34:16Z

Awaiting bors try build completion

bors · 2020-01-31T23:34:26Z

⌛ Trying commit f52b65c85a861033e46576cf9853683c6ba47ff8 with merge 89c6ff362254e2735d7304447a89dca0f130da31...

bors · 2020-02-01T02:37:59Z

☀️ Try build successful - checks-azure
Build commit: 89c6ff362254e2735d7304447a89dca0f130da31 (89c6ff362254e2735d7304447a89dca0f130da31)

rust-timer · 2020-02-01T02:38:01Z

Queued 89c6ff362254e2735d7304447a89dca0f130da31 with parent cd1ef39, future comparison URL.

michaelwoerister · 2020-02-03T16:17:16Z

The performance numbers look good. I'll try to make time for reviewing this some time this or next week.

michaelwoerister

I'm generally in favor of going forward with this. It's a good idea to move some of the more expensive operations out of the fast-path. I have one main concern that I listed below.

src/librustc/ty/query/job.rs

michaelwoerister · 2020-02-12T12:46:51Z

src/librustc/ty/query/plumbing.rs

@@ -139,39 +149,54 @@ impl<'a, 'tcx, Q: QueryDescription<'tcx>> JobOwner<'a, 'tcx, Q> {
                                query_blocked_prof_timer = Some(tcx.prof.query_blocked());
                            }

-                            job.clone()
+                            // Create the id of the job we're waiting for
+                            let id = QueryJobId {


Would you mind creating a helper method for this? This function is already rather big.

src/librustc/ty/query/plumbing.rs

michaelwoerister · 2020-02-12T12:50:23Z

src/librustc/ty/query/plumbing.rs

-        key: &Q::Key,
-        token: QueryToken,
-    ) -> TryGetJob<'a, 'tcx, Q> {
+    pub(super) fn try_get(tcx: TyCtxt<'tcx>, span: Span, key: &Q::Key) -> TryGetJob<'a, 'tcx, Q> {


I like how token disappears from the function signature again.

michaelwoerister · 2020-02-13T14:47:26Z

Thanks, @Zoxc!

@bors r+ rollup=never

(This is doing some complicated changes, let's keep it out of rollups in case it breaks anything)

bors · 2020-02-13T14:47:28Z

📌 Commit 5206827 has been approved by michaelwoerister

bors · 2020-02-13T15:56:40Z

⌛ Testing commit 5206827 with merge 9503b49...

@michaelwoerister

Construct query job latches on-demand r? @michaelwoerister

bors · 2020-02-13T19:12:38Z

💔 Test failed - checks-azure

Zoxc · 2020-02-13T20:06:28Z

@bors retry

Dylan-DPC-zz · 2020-02-14T01:34:33Z

@bors p=1

bors · 2020-02-14T01:37:55Z

⌛ Testing commit 5206827 with merge 21ed505...

@michaelwoerister

Construct query job latches on-demand r? @michaelwoerister

bors · 2020-02-14T04:50:01Z

☀️ Test successful - checks-azure
Approved by: michaelwoerister
Pushing 21ed505 to master...

rust-highfive · 2020-02-14T04:51:16Z

📣 Toolstate changed by #68693!

Tested on commit 21ed505.
Direct link to PR: #68693

💔 rustc-guide on linux: test-pass → test-fail (cc @JohnTitor @amanjeev @spastorino @mark-i-m, @rust-lang/infra).

@JohnTitor

Tested on commit rust-lang/rust@21ed505. Direct link to PR: <rust-lang/rust#68693> 💔 rustc-guide on linux: test-pass → test-fail (cc @JohnTitor @amanjeev @spastorino @mark-i-m, @rust-lang/infra).

rust-highfive assigned michaelwoerister Jan 31, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 31, 2020

Zoxc force-pushed the query-no-arc branch from aa0abc8 to 8ecd740 Compare January 31, 2020 06:30

bors added a commit that referenced this pull request Jan 31, 2020

Auto merge of #68693 - Zoxc:query-no-arc, r=<try>

7ccaf1c

[WIP] Construct query job latches on-demand r? @michaelwoerister

Zoxc force-pushed the query-no-arc branch from 8ecd740 to f52b65c Compare January 31, 2020 20:50

Zoxc changed the title ~~[WIP] Construct query job latches on-demand~~ Construct query job latches on-demand Jan 31, 2020

Zoxc mentioned this pull request Feb 2, 2020

Move the dep_graph construction to a dedicated crate. #67761

Merged

michaelwoerister reviewed Feb 11, 2020

View reviewed changes

src/librustc/ty/query/job.rs Outdated Show resolved Hide resolved

Zoxc added 2 commits February 12, 2020 10:30

Construct query job latches on-demand

77ab0d0

Use a counter instead of pointers to the stack

19c1012

Zoxc force-pushed the query-no-arc branch from f52b65c to 19c1012 Compare February 12, 2020 10:50

Drop the lock guard

5de82b9

michaelwoerister reviewed Feb 12, 2020

View reviewed changes

src/librustc/ty/query/plumbing.rs Outdated Show resolved Hide resolved

michaelwoerister reviewed Feb 12, 2020

View reviewed changes

Address comments

5206827

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 13, 2020

bors added a commit that referenced this pull request Feb 13, 2020

Auto merge of #68693 - Zoxc:query-no-arc, r=michaelwoerister

9503b49

Construct query job latches on-demand r? @michaelwoerister

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

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 13, 2020

bors added a commit that referenced this pull request Feb 14, 2020

Auto merge of #68693 - Zoxc:query-no-arc, r=michaelwoerister

21ed505

Construct query job latches on-demand r? @michaelwoerister

bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 14, 2020

bors merged commit 5206827 into rust-lang:master Feb 14, 2020

bors mentioned this pull request Feb 14, 2020

Add an abstraction for custom query caches #68988

Merged

This was referenced Feb 14, 2020

Use queries for the HIR map #68944

Merged

Make dep node indices persistent between sessions #62038

Closed

Zoxc deleted the query-no-arc branch February 14, 2020 08:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Construct query job latches on-demand #68693

Construct query job latches on-demand #68693

Zoxc commented Jan 31, 2020

Zoxc commented Jan 31, 2020

bors commented Jan 31, 2020

Zoxc commented Jan 31, 2020

rust-timer commented Jan 31, 2020

bors commented Jan 31, 2020

rust-highfive commented Jan 31, 2020

bors commented Jan 31, 2020

rust-timer commented Jan 31, 2020

rust-timer commented Jan 31, 2020

Zoxc commented Jan 31, 2020

Zoxc commented Jan 31, 2020

rust-timer commented Jan 31, 2020

bors commented Jan 31, 2020

bors commented Feb 1, 2020

rust-timer commented Feb 1, 2020

michaelwoerister commented Feb 3, 2020

michaelwoerister left a comment

michaelwoerister Feb 12, 2020

Zoxc Feb 12, 2020

michaelwoerister Feb 12, 2020

michaelwoerister commented Feb 13, 2020

bors commented Feb 13, 2020

bors commented Feb 13, 2020

bors commented Feb 13, 2020

Zoxc commented Feb 13, 2020

Dylan-DPC-zz commented Feb 14, 2020

bors commented Feb 14, 2020

bors commented Feb 14, 2020

rust-highfive commented Feb 14, 2020

Construct query job latches on-demand #68693

Construct query job latches on-demand #68693

Conversation

Zoxc commented Jan 31, 2020

Zoxc commented Jan 31, 2020

bors commented Jan 31, 2020

Zoxc commented Jan 31, 2020

rust-timer commented Jan 31, 2020

bors commented Jan 31, 2020

rust-highfive commented Jan 31, 2020

bors commented Jan 31, 2020

rust-timer commented Jan 31, 2020

rust-timer commented Jan 31, 2020

Zoxc commented Jan 31, 2020

Zoxc commented Jan 31, 2020

rust-timer commented Jan 31, 2020

bors commented Jan 31, 2020

bors commented Feb 1, 2020

rust-timer commented Feb 1, 2020

michaelwoerister commented Feb 3, 2020

michaelwoerister left a comment

Choose a reason for hiding this comment

michaelwoerister Feb 12, 2020

Choose a reason for hiding this comment

Zoxc Feb 12, 2020

Choose a reason for hiding this comment

michaelwoerister Feb 12, 2020

Choose a reason for hiding this comment

michaelwoerister commented Feb 13, 2020

bors commented Feb 13, 2020

bors commented Feb 13, 2020

bors commented Feb 13, 2020

Zoxc commented Feb 13, 2020

Dylan-DPC-zz commented Feb 14, 2020

bors commented Feb 14, 2020

bors commented Feb 14, 2020

rust-highfive commented Feb 14, 2020