incr.comp.: Use a set implementation optimized for small item counts for deduplicating read-edges.

[michaelwoerister]

Many kinds of DepNodes will only ever have between zero and three edges originating from them (see e.g. #45063 (comment)) so let's try to avoid allocating a HashSet in those cases.

r? @nikomatsakis

[leonardo-m]

Something like this? https://crates.io/crates/vec_map

[kennytm]

@leonardo-m No, this structure is more like https://docs.rs/david-set/0.1.2/david_set/struct.Set.html.

[arielb1]

Could you move this to a generic data structure in rustc_data_structures?

[arielb1]

Could you try and get some numbers on the performance improvement or the hit rate?

Even if you don't, r=me with the set moved to rustc_data_structures.

[kennytm]

@bors try

Preparing for perf.

[bors]

⌛ Trying commit be27d8b with merge 877833f...

[bors]

☀️ Test successful - status-travis
State: approved= try=True

[kennytm]

@rust-lang/infra perf check requested from #45577 (comment).

[arthurprs]

This is fine, but an enum InlineSet<T> {Inline(ArrayVec<T>), Extended(FxHashSet<T>) } would be nicer and reusable.

Note: ArrayVec is already in the codebase.

[Mark-Simulacrum]

http://perf.rust-lang.org/compare.html?start=7da9a5e178e28b2e387e6296aa1b0289acdf5781&end=877833ffc410fea76cf3eb1bee275ccb92a43e7d

[kennytm]

The improvement/regression is negligible. There is even a +17.4% max-rss (memory use) in the regex-opt-0.1.80@020-incr-from... test case. Overall landing this PR does not seem beneficial.

Measure	∑before	∑after	Δ
cpu-clock	414267.47	411066.66	-0.77%
cycles:u	1582921482931	1570998057890	-0.75%
faults	1842937	1836494	-0.35%
instructions:u	1832233383606	1832161663635	-0.00%
max-rss	17936396	18100956	+0.92%
task-clock	413244.27	409110.92	-1.00%
wall-time	220.22	220.70	+0.22%

[michaelwoerister]

Thanks for kicking off the performance measurement, @kennytm!

Could you try and get some numbers on the performance improvement or the hit rate?

@julian-seward1 and I ran into this function as on of the hotter ones while profiling a incremental building of the regex crate. The compiler spends roughly 0.9% and 1.6% of cycles in the read_index() function, depending on how much of the dependency graph can be marked as green. So that's the upper limit of the performance improvement here.

The main intended optimization here is to get rid of the heap allocation for small hash sets. It's a bit surprising that the effect on the overall instruction count is so small. I would have hoped for -0.5% instead of -0.1%.

There is even a +17.4% max-rss (memory use) in the regex-opt-0.1.80@020-incr-from... test case.

This must be some other effect. I don't see how this could introduce any noticeable increase in memory consumption.

Thanks for your comments, everyone! Maybe I'll tinker some more with this in my spare time. Closing for now.

[kennytm]

The main intended optimization here is to get rid of the heap allocation for small hash sets. It's a bit surprising that the effect on the overall instruction count is so small. I would have hoped for -0.5% instead of -0.1%.

@michaelwoerister Probably it's because jemalloc is very efficient? 😄