@wg-incr-comp: I was watching the last meeting's video, and a status update on #79519 is useful.
The objective of the PR is to remove all the
&[Attribute] slices from the HIR.
The operational assumption is that attributes only impact one pass of the compilation process (eg: allow one lint). Therefore, removing them from the HIR would allow to reuse a lot of analyses.
Technically, the attributes are stored out of the HIR tree in a
HirId -> &[Attribute] map. This map is accessible through a
hir_attrs(HIR owner) query. The commits modify the HIR nodes one by one along with the fallout in other crates. A few commits contain refactorings, btu not nearly enough.
The first version used nested
IndexVecs, and had a ~5% instruction-count regression.
The latest version uses a
BTreeMap<HirId, &[Attribute]>, considering attributes are rare and this map should be sparse. The instruction-count regression is at ~2%.
I am out of ideas on how to remove this perf regression.
ThinIndexVec idea unlikely to have a better performance profile than the
ThinIndexVecidea unlikely to have a better performance profile than the
To expand on this, the idea was what I mentioned here and to try using a
IndexVec<LocalDefId, ThinIndexVec<ItemLocalId, &'tcx [Attribute]>> instead of the original
IndexVec<LocalDefId, IndexVec<ItemLocalId, &'tcx [Attribute]>> which is now a
BTreeMap. We could then compare the
BTreeMap performance vs the
IndexVec<LocalDefId, ThinIndexVec<ItemLocalId, &'tcx [Attribute]>> performance and see which is better.
I haven't tried ThinIndexVec yet (missed that part of your comment).
I am not sure it would help: going from
ThinIndex is a pessimisation in terms of instruction count.
BTreeMap is already less-bad in instr-count, and a win in max-rss.
Nevertheless, I will try the ThinIndexVec just to be sure.
No worries! The
BTreeMap helped a lot more than I thought it would but it would still be interesting to compare with the
Perf results for ThinIndexVec in https://perf.rust-lang.org/compare.html?start=07194ffcd25b0871ce560b9f702e52db27ac9f77&end=f45ada695c0cefd9467ffed78545a63ef2341bbd
Perf regression ~6%, a bit worse than the original IndexVec version. No significant difference on max-rss.
Last updated: Oct 21 2021 at 20:03 UTC