Refactor item attributes into a SortedIndexMultiMap<u32, Symbol, &Attribute>

[cjgillot]

Now, when code needs to look up attributes on an item, it fetches the complete list of attributes and sequentially looks for the interesting one. This could be made more efficient by making the attribute list a SortedIndexMultiMap.

Steps:

• replace &[ast::Attribute] by SortedIndexMultiMap<u32, Symbol, &ast::Attribute> in rustc_hir::hir, rustc_middle::query and rustc_middle::ty;
• build the SortedIndexMultiMap in rustc_ast_lowering and rustc_metadata::decoder using Attribute::name_or_empty as key;
• adapt the use sites to fetch the attributes using attrs.get_by_key(my_name) instead of attrs.iter().filter(|attr| attr.has_name(my_name);
• evaluate performance impact.

Extra:

• implement item_attrs query for the local crate, and replace calls to tcx.get_attrs by calls to tcx.item_attrs.

I am available on zulip for more detailed information.

[camelid]

I would like to attempt this. @rustbot claim

[raiyansayeed]

Hey @camelid , just wondering if you're still working on the task?

[NEUDitao]

Since @camelid hasn't reported on this in a while, can I take a stab at it?

[camelid]

Hi, sorry, I didn't see these previous messages. I did attempt this but got stuck and didn't have time to figure out the right approach. Feel free to try it!

[NEUDitao]

@rustbot claim

[hottea773]

@NEUDitao are you actively working on this?

I had a little look at this, but hit an issue in replacing &[ast::Attribute] with SortedIndexMultiMap<u32, Symbol, &ast::Attribute> in rustc_middle::query; specifically, https://github.com/rust-lang/rust/blob/1.63.0/compiler/rustc_middle/src/query/mod.rs#L1105.

error[E0277]: the trait bound `rustc_data_structures::sorted_map::SortedIndexMultiMap<u32, rustc_span::Symbol, &Attribute>: std::marker::Copy` is not satisfied
    --> compiler/rustc_middle/src/ty/query.rs:255:91
     |
178  | /  macro_rules! define_callbacks {
179  | |      (
180  | |       $($(#[$attr:meta])*
181  | |          [$($modifiers:tt)*] fn $name:ident($($K:tt)*) -> $V:ty,)*) => {
...    |
255  | |                  let cached = try_get_cached(self.tcx, &self.tcx.query_caches.$name, &key, copy);
     | |                                                                                            ^^^^ the trait `std::marker::Copy` is not implemented for `rustc_data_structures::sorted_map::SortedIndexMultiMap<u32, rustc_span::Symbol, &Attribute>`
...    |
324  | |      };
325  | |  }
     | |__- in this expansion of `define_callbacks!` (#2)
...
339  |    rustc_query_append! { define_callbacks! }
     |    ----------------------------------------- in this macro invocation (#1)
     |
    ::: compiler/rustc_middle/src/query/mod.rs:18:1
     |
18   |  / rustc_queries! {
19   |  |     query trigger_delay_span_bug(key: DefId) -> () {
20   |  |         desc { "trigger a delay span bug" }
21   |  |     }
...     |
2101 |  |     }
2102 |  | }
     |  | -
     |  | |
     |  |_in this expansion of `rustc_query_append!` (#1)
     |    in this macro invocation (#2)
     |
note: required by a bound in `ty::query::copy`
    --> compiler/rustc_middle/src/ty/query.rs:114:12
     |
114  | fn copy<T: Copy>(x: &T) -> T {
     |            ^^^^ required by this bound in `ty::query::copy`

Which is deliberate; as in https://github.com/rust-lang/rust/blob/1.63.0/compiler/rustc_middle/src/ty/query.rs#L108-L112.
I think this means this approach doesn't work; but I'm a total newbie to rustc development, so am not sure whether there are larger things we should be doing to get this working.

[cjgillot]

@hottea773 Query results indeed require to be Copy. If the information you are constructing is not Copy, you can use arena_cache in the query description. The computed value will be put on an arena, and the query will just return a reference to it, which is Copy.

[czzrr]

@rustbot claim

[Jesse-Bakker]

Hey @czzrr, are you actively working on this? Otherwise I'll take a stab at it

[czzrr]

@Jesse-Bakker Go ahead - I don't have the time right now :)

[Jesse-Bakker]

@rustbot claim

[cjgillot]

Closing as this did not yield the wanted improvement in #109532.