From afa569dc4e42a44ce9a2aa0152357869112c247c Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 2 Feb 2023 05:34:10 +0000 Subject: [PATCH 1/5] try using a non-generic closure in `start_query` --- compiler/rustc_query_impl/src/plumbing.rs | 2 +- compiler/rustc_query_system/src/query/mod.rs | 2 +- compiler/rustc_query_system/src/query/plumbing.rs | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index 005ce16dbb9b4..9f904ba935c98 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -104,7 +104,7 @@ impl QueryContext for QueryCtxt<'_> { token: QueryJobId, depth_limit: bool, diagnostics: Option<&Lock>>, - compute: impl FnOnce() -> R, + compute: &mut dyn FnMut() -> R, ) -> R { // The `TyCtxt` stored in TLS has the same global interner lifetime // as `self`, so we use `with_related_context` to relate the 'tcx lifetimes diff --git a/compiler/rustc_query_system/src/query/mod.rs b/compiler/rustc_query_system/src/query/mod.rs index 312b0e1688dc9..28a9eab8ad5a6 100644 --- a/compiler/rustc_query_system/src/query/mod.rs +++ b/compiler/rustc_query_system/src/query/mod.rs @@ -128,7 +128,7 @@ pub trait QueryContext: HasDepContext { token: QueryJobId, depth_limit: bool, diagnostics: Option<&Lock>>, - compute: impl FnOnce() -> R, + compute: &mut dyn FnMut() -> R, ) -> R; fn depth_limit_error(self, job: QueryJobId); diff --git a/compiler/rustc_query_system/src/query/plumbing.rs b/compiler/rustc_query_system/src/query/plumbing.rs index 005fcd8c4cc9d..90f109dd41ee1 100644 --- a/compiler/rustc_query_system/src/query/plumbing.rs +++ b/compiler/rustc_query_system/src/query/plumbing.rs @@ -421,7 +421,7 @@ where } let prof_timer = qcx.dep_context().profiler().query_provider(); - let result = qcx.start_query(job_id, query.depth_limit(), None, || query.compute(qcx, key)); + let result = qcx.start_query(job_id, query.depth_limit(), None, &mut || query.compute(qcx, key)); let dep_node_index = dep_graph.next_virtual_depnode_index(); prof_timer.finish_with_query_invocation_id(dep_node_index.into()); @@ -445,7 +445,7 @@ where // The diagnostics for this query will be promoted to the current session during // `try_mark_green()`, so we can ignore them here. - if let Some(ret) = qcx.start_query(job_id, false, None, || { + if let Some(ret) = qcx.start_query(job_id, false, None, &mut || { try_load_from_disk_and_cache_in_memory(query, qcx, &key, &dep_node) }) { return ret; @@ -456,7 +456,7 @@ where let diagnostics = Lock::new(ThinVec::new()); let (result, dep_node_index) = - qcx.start_query(job_id, query.depth_limit(), Some(&diagnostics), || { + qcx.start_query(job_id, query.depth_limit(), Some(&diagnostics), &mut || { if query.anon() { return dep_graph.with_anon_task(*qcx.dep_context(), query.dep_kind(), || { query.compute(qcx, key) From 390fd194f45d375c2a136afa949293c3534f2e53 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 1 Mar 2023 22:12:41 -0600 Subject: [PATCH 2/5] Revert "try using a non-generic closure in `start_query`" This reverts commit 279063d6a14269c94d56258fd411f4059643e20a. --- compiler/rustc_query_impl/src/plumbing.rs | 2 +- compiler/rustc_query_system/src/query/mod.rs | 2 +- compiler/rustc_query_system/src/query/plumbing.rs | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index 9f904ba935c98..005ce16dbb9b4 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -104,7 +104,7 @@ impl QueryContext for QueryCtxt<'_> { token: QueryJobId, depth_limit: bool, diagnostics: Option<&Lock>>, - compute: &mut dyn FnMut() -> R, + compute: impl FnOnce() -> R, ) -> R { // The `TyCtxt` stored in TLS has the same global interner lifetime // as `self`, so we use `with_related_context` to relate the 'tcx lifetimes diff --git a/compiler/rustc_query_system/src/query/mod.rs b/compiler/rustc_query_system/src/query/mod.rs index 28a9eab8ad5a6..312b0e1688dc9 100644 --- a/compiler/rustc_query_system/src/query/mod.rs +++ b/compiler/rustc_query_system/src/query/mod.rs @@ -128,7 +128,7 @@ pub trait QueryContext: HasDepContext { token: QueryJobId, depth_limit: bool, diagnostics: Option<&Lock>>, - compute: &mut dyn FnMut() -> R, + compute: impl FnOnce() -> R, ) -> R; fn depth_limit_error(self, job: QueryJobId); diff --git a/compiler/rustc_query_system/src/query/plumbing.rs b/compiler/rustc_query_system/src/query/plumbing.rs index 90f109dd41ee1..005fcd8c4cc9d 100644 --- a/compiler/rustc_query_system/src/query/plumbing.rs +++ b/compiler/rustc_query_system/src/query/plumbing.rs @@ -421,7 +421,7 @@ where } let prof_timer = qcx.dep_context().profiler().query_provider(); - let result = qcx.start_query(job_id, query.depth_limit(), None, &mut || query.compute(qcx, key)); + let result = qcx.start_query(job_id, query.depth_limit(), None, || query.compute(qcx, key)); let dep_node_index = dep_graph.next_virtual_depnode_index(); prof_timer.finish_with_query_invocation_id(dep_node_index.into()); @@ -445,7 +445,7 @@ where // The diagnostics for this query will be promoted to the current session during // `try_mark_green()`, so we can ignore them here. - if let Some(ret) = qcx.start_query(job_id, false, None, &mut || { + if let Some(ret) = qcx.start_query(job_id, false, None, || { try_load_from_disk_and_cache_in_memory(query, qcx, &key, &dep_node) }) { return ret; @@ -456,7 +456,7 @@ where let diagnostics = Lock::new(ThinVec::new()); let (result, dep_node_index) = - qcx.start_query(job_id, query.depth_limit(), Some(&diagnostics), &mut || { + qcx.start_query(job_id, query.depth_limit(), Some(&diagnostics), || { if query.anon() { return dep_graph.with_anon_task(*qcx.dep_context(), query.dep_kind(), || { query.compute(qcx, key) From bbed70ce578e067b6b4979eb474141ca825a9500 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 1 Mar 2023 22:15:58 -0600 Subject: [PATCH 3/5] Try using `inline(always)` to avoid a runtime regression --- compiler/rustc_middle/src/ty/context/tls.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_middle/src/ty/context/tls.rs b/compiler/rustc_middle/src/ty/context/tls.rs index 5426ac8d73992..41e0454107d2d 100644 --- a/compiler/rustc_middle/src/ty/context/tls.rs +++ b/compiler/rustc_middle/src/ty/context/tls.rs @@ -115,7 +115,7 @@ where } /// Allows access to the current `ImplicitCtxt` in a closure if one is available. -#[inline] +#[inline(always)] pub fn with_context_opt(f: F) -> R where F: for<'a, 'tcx> FnOnce(Option<&ImplicitCtxt<'a, 'tcx>>) -> R, @@ -134,7 +134,7 @@ where /// Allows access to the current `ImplicitCtxt`. /// Panics if there is no `ImplicitCtxt` available. -#[inline] +#[inline(always)] pub fn with_context(f: F) -> R where F: for<'a, 'tcx> FnOnce(&ImplicitCtxt<'a, 'tcx>) -> R, @@ -147,7 +147,7 @@ where /// as the `TyCtxt` passed in. /// This will panic if you pass it a `TyCtxt` which is different from the current /// `ImplicitCtxt`'s `tcx` field. -#[inline] +#[inline(always)] pub fn with_related_context<'tcx, F, R>(tcx: TyCtxt<'tcx>, f: F) -> R where F: FnOnce(&ImplicitCtxt<'_, 'tcx>) -> R, From 6322be36c828a7e0af0682ebfa008611550b438b Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 2 Feb 2023 05:34:10 +0000 Subject: [PATCH 4/5] combine both approaches --- compiler/rustc_query_impl/src/plumbing.rs | 2 +- compiler/rustc_query_system/src/query/mod.rs | 2 +- compiler/rustc_query_system/src/query/plumbing.rs | 7 ++++--- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index 005ce16dbb9b4..9f904ba935c98 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -104,7 +104,7 @@ impl QueryContext for QueryCtxt<'_> { token: QueryJobId, depth_limit: bool, diagnostics: Option<&Lock>>, - compute: impl FnOnce() -> R, + compute: &mut dyn FnMut() -> R, ) -> R { // The `TyCtxt` stored in TLS has the same global interner lifetime // as `self`, so we use `with_related_context` to relate the 'tcx lifetimes diff --git a/compiler/rustc_query_system/src/query/mod.rs b/compiler/rustc_query_system/src/query/mod.rs index 312b0e1688dc9..28a9eab8ad5a6 100644 --- a/compiler/rustc_query_system/src/query/mod.rs +++ b/compiler/rustc_query_system/src/query/mod.rs @@ -128,7 +128,7 @@ pub trait QueryContext: HasDepContext { token: QueryJobId, depth_limit: bool, diagnostics: Option<&Lock>>, - compute: impl FnOnce() -> R, + compute: &mut dyn FnMut() -> R, ) -> R; fn depth_limit_error(self, job: QueryJobId); diff --git a/compiler/rustc_query_system/src/query/plumbing.rs b/compiler/rustc_query_system/src/query/plumbing.rs index 005fcd8c4cc9d..c4c044a2d380d 100644 --- a/compiler/rustc_query_system/src/query/plumbing.rs +++ b/compiler/rustc_query_system/src/query/plumbing.rs @@ -421,7 +421,8 @@ where } let prof_timer = qcx.dep_context().profiler().query_provider(); - let result = qcx.start_query(job_id, query.depth_limit(), None, || query.compute(qcx, key)); + let result = + qcx.start_query(job_id, query.depth_limit(), None, &mut || query.compute(qcx, key)); let dep_node_index = dep_graph.next_virtual_depnode_index(); prof_timer.finish_with_query_invocation_id(dep_node_index.into()); @@ -445,7 +446,7 @@ where // The diagnostics for this query will be promoted to the current session during // `try_mark_green()`, so we can ignore them here. - if let Some(ret) = qcx.start_query(job_id, false, None, || { + if let Some(ret) = qcx.start_query(job_id, false, None, &mut || { try_load_from_disk_and_cache_in_memory(query, qcx, &key, &dep_node) }) { return ret; @@ -456,7 +457,7 @@ where let diagnostics = Lock::new(ThinVec::new()); let (result, dep_node_index) = - qcx.start_query(job_id, query.depth_limit(), Some(&diagnostics), || { + qcx.start_query(job_id, query.depth_limit(), Some(&diagnostics), &mut || { if query.anon() { return dep_graph.with_anon_task(*qcx.dep_context(), query.dep_kind(), || { query.compute(qcx, key) From 5827f79dd56ca7a1b3646457ae064f4cc4451324 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 9 Mar 2023 04:19:25 +0000 Subject: [PATCH 5/5] Revert "Try using `inline(always)` to avoid a runtime regression" This reverts commit bbed70ce578e067b6b4979eb474141ca825a9500. --- compiler/rustc_middle/src/ty/context/tls.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_middle/src/ty/context/tls.rs b/compiler/rustc_middle/src/ty/context/tls.rs index 41e0454107d2d..5426ac8d73992 100644 --- a/compiler/rustc_middle/src/ty/context/tls.rs +++ b/compiler/rustc_middle/src/ty/context/tls.rs @@ -115,7 +115,7 @@ where } /// Allows access to the current `ImplicitCtxt` in a closure if one is available. -#[inline(always)] +#[inline] pub fn with_context_opt(f: F) -> R where F: for<'a, 'tcx> FnOnce(Option<&ImplicitCtxt<'a, 'tcx>>) -> R, @@ -134,7 +134,7 @@ where /// Allows access to the current `ImplicitCtxt`. /// Panics if there is no `ImplicitCtxt` available. -#[inline(always)] +#[inline] pub fn with_context(f: F) -> R where F: for<'a, 'tcx> FnOnce(&ImplicitCtxt<'a, 'tcx>) -> R, @@ -147,7 +147,7 @@ where /// as the `TyCtxt` passed in. /// This will panic if you pass it a `TyCtxt` which is different from the current /// `ImplicitCtxt`'s `tcx` field. -#[inline(always)] +#[inline] pub fn with_related_context<'tcx, F, R>(tcx: TyCtxt<'tcx>, f: F) -> R where F: FnOnce(&ImplicitCtxt<'_, 'tcx>) -> R,