Skip to content

Several query cleanups #104023

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 5 commits into from
Nov 8, 2022
Merged

Several query cleanups #104023

merged 5 commits into from
Nov 8, 2022

Conversation

Noratrieb
Copy link
Member

A few cleanups, mostly about naming in rustc_query_system.

r? @cjgillot

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) 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. labels Nov 5, 2022
@bors
Copy link
Collaborator

bors commented Nov 6, 2022

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

Copy link
Contributor

@cjgillot cjgillot left a comment

Choose a reason for hiding this comment

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

Just one nit.
r=me after rebase.

@@ -26,7 +26,7 @@ impl<Key, Value> Cache<Key, Value> {
}

impl<Key: Eq + Hash, Value: Clone> Cache<Key, Value> {
pub fn get<CTX: DepContext>(&self, key: &Key, tcx: CTX) -> Option<Value> {
pub fn get<Tcx: DepContext>(&self, key: &Key, tcx: Tcx) -> Option<Value> {
Copy link
Contributor

Choose a reason for hiding this comment

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

If we really want to be consistent, this would be a Dcx for DepContext, but I don't mind.

@cjgillot
Copy link
Contributor

cjgillot commented Nov 6, 2022

Query system is perf sensitive. There shouldn't really be anything, but just in case:
@bors r+ rollup=never

@bors
Copy link
Collaborator

bors commented Nov 6, 2022

📌 Commit 6d26ea8 has been approved by cjgillot

It is now in the queue for this repository.

@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 6, 2022
@bors
Copy link
Collaborator

bors commented Nov 6, 2022

⌛ Testing commit 6d26ea8 with merge 310880ff180830e77647aa6eacaca94f42ab201d...

@bors
Copy link
Collaborator

bors commented Nov 6, 2022

💔 Test failed - checks-actions

@bors 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 Nov 6, 2022
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-apple-2 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
      Memory: 14 GB
      System Firmware Version: VMW71.00V.13989454.B64.1906190538
      Apple ROM Info: [MS_VM_CERT/SHA1/27d66596a61c48dd3dc7216fd715126e33f59ae7]Welcome to the Virtual Machine
      SMC Version (system): 2.8f0
      Serial Number (system): VMNFZU8uy8KA
      Provisioning UDID: 4203018E-580F-C1B5-9525-B745CECA79EB

hw.ncpu: 3
hw.byteorder: 1234
---
[  0%] Built target RTLSanCommon.osx
[  0%] Built target RTSanitizerCommonLibc.osx
Consolidate compiler generated dependencies of target RTSanitizerCommonCoverage.osx
[ 57%] Built target RTSanitizerCommon.osx
lib/sanitizer_common/CMakeFiles/RTSanitizerCommonSymbolizer.osx.dir/compiler_depend.make:751: *** multiple target patterns.  Stop.
[ 57%] Built target RTSanitizerCommonCoverage.osx
make[2]: *** [lib/sanitizer_common/CMakeFiles/RTSanitizerCommonSymbolizer.osx.dir/all] Error 2
make[2]: *** [lib/sanitizer_common/CMakeFiles/RTSanitizerCommonSymbolizer.osx.dir/all] Error 2
make[2]: *** Waiting for unfinished jobs....
[ 57%] Built target RTInterception.osx
[ 57%] Built target RTInterception.osx
make[1]: *** [lib/lsan/CMakeFiles/clang_rt.lsan_osx_dynamic.dir/rule] Error 2
make: *** [clang_rt.lsan_osx_dynamic] Error 2
command did not execute successfully, got: exit status: 2


build script failed, must exit now', /Users/runner/.cargo/registry/src/github.heygears.com-1ecc6299db9ec823/cmake-0.1.48/src/lib.rs:975:5
 finished in 109.503 seconds
Build completed unsuccessfully in 0:43:07

@Noratrieb
Copy link
Member Author

Looks spurious..

@Dylan-DPC
Copy link
Member

@bors retry

@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 7, 2022
@bors
Copy link
Collaborator

bors commented Nov 8, 2022

⌛ Testing commit 6d26ea8 with merge 6b23a7e...

@bors
Copy link
Collaborator

bors commented Nov 8, 2022

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing 6b23a7e to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 8, 2022
@bors bors merged commit 6b23a7e into rust-lang:master Nov 8, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (6b23a7e): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.4% [0.4%, 0.4%] 1
Regressions ❌
(secondary)
7.6% [7.6%, 7.6%] 1
Improvements ✅
(primary)
-0.9% [-1.0%, -0.8%] 2
Improvements ✅
(secondary)
-3.3% [-3.3%, -3.3%] 1
All ❌✅ (primary) -0.4% [-1.0%, 0.4%] 3

Cycles

This benchmark run did not return any relevant results for this metric.

@Noratrieb Noratrieb deleted the cleanup-query branch November 8, 2022 08:30
@Noratrieb Noratrieb restored the cleanup-query branch December 3, 2022 19:44
@Noratrieb Noratrieb deleted the cleanup-query branch December 23, 2022 21:03
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

7 participants