Add secondary stress benchmark for adding trait in owner's trait_map#2436
Add secondary stress benchmark for adding trait in owner's trait_map#2436aerooneqq wants to merge 1 commit into
trait_map#2436Conversation
aerooneqq
force-pushed
the
add-trait-incr-patches
branch
from
5c25208 to
569c00d
Compare
|
Looks simple enough, and should be fast to build. @nnethercote Any concerns? |
|
I think if rust-lang/rust#155678 makes the new benchmark red in the incr-changed scenario, we should still merge it. Code changes between two incremental builds do not typically include adding a trait (import) with specific item name. |
Yes, adding traits does seem like a rare scenario so I don't feel the need to for a benchmark to test it. I won't block it if someone else is strongly in favour, but it seems fairly low value. |
On my laptop it shows the following numbers (with regressions in incr-patched scenario). But they are not as big as it was in first perf run. |
aerooneqq
force-pushed
the
add-trait-incr-patches
branch
from
569c00d to
bb82f70
Compare
Merge several HIR-level queries into one Now four queries (`local_def_id_to_hir_id`, `opt_hir_owner_nodes`, `opt_ast_lowering_delayed_lints`, `in_scope_traits_map`) were replaced with regular methods which acts like getters. An `hir_owner` query was added that returns a `ProjectedMaybeOwner` that contains all those fields. `hir_attr_map` remains a separate query as adding attributes to `ProjectedMaybeOwner` led to [perf regressions](#155678 (comment)). There is a similar issue with `in_scopes_trait_map`, but according to the comments from rust-lang/rustc-perf#2436 it is a rare case. Most of the changes in incremental tests are renames from `opt_hir_owner_nodes` -> `hir_owner`, but there are few cases when new dirty queries were added. r? @petrochenkov r? @oli-obk
|
Closing as #155678 is merged. |
Merge several HIR-level queries into one Now four queries (`local_def_id_to_hir_id`, `opt_hir_owner_nodes`, `opt_ast_lowering_delayed_lints`, `in_scope_traits_map`) were replaced with regular methods which acts like getters. An `hir_owner` query was added that returns a `ProjectedMaybeOwner` that contains all those fields. `hir_attr_map` remains a separate query as adding attributes to `ProjectedMaybeOwner` led to [perf regressions](rust-lang/rust#155678 (comment)). There is a similar issue with `in_scopes_trait_map`, but according to the comments from rust-lang/rustc-perf#2436 it is a rare case. Most of the changes in incremental tests are renames from `opt_hir_owner_nodes` -> `hir_owner`, but there are few cases when new dirty queries were added. r? @petrochenkov r? @oli-obk
Merge several HIR-level queries into one Now four queries (`local_def_id_to_hir_id`, `opt_hir_owner_nodes`, `opt_ast_lowering_delayed_lints`, `in_scope_traits_map`) were replaced with regular methods which acts like getters. An `hir_owner` query was added that returns a `ProjectedMaybeOwner` that contains all those fields. `hir_attr_map` remains a separate query as adding attributes to `ProjectedMaybeOwner` led to [perf regressions](rust-lang/rust#155678 (comment)). There is a similar issue with `in_scopes_trait_map`, but according to the comments from rust-lang/rustc-perf#2436 it is a rare case. Most of the changes in incremental tests are renames from `opt_hir_owner_nodes` -> `hir_owner`, but there are few cases when new dirty queries were added. r? @petrochenkov r? @oli-obk
4 participants
This PR adds a new secondary benchmark for case when a new trait is added to
in_scope_traits_mapquery. During experiments with queries in #155678 (where I merged some queries into one, meaning invalidation of any part of the query will invalidate whole new merged query) improvements were reported, however I did not find any benchmark that covered case when a new trait is added toin_scope_traits_mapquery, so I can not fully trust those results.A real-life scenario where such regression is possible is importing (or creating) a new trait with functions whose names are mentioned frequently in a file in a deferred typecheck context.
cc @petrochenkov