Use a single map for the CurrentDepGraph

[Mark-Simulacrum]

An IndexMap instead of a HashMap and IndexVec is used. Internally this is much the same, but wrapping the two into one data structure is less prone to accidents of updating just one and is cleaner. Big-O lookups are the same as previously for DepNode/DepNodeIndex, so performance wise this is not expected to have significant effect. IndexMap retains the same cache characteristics though at our size it probably has little to no effect.

Changes here are pretty straightforward, though could be made even more so with an addition to librustc_index that exposes a wrapper around IndexMap which seamlessly presents Idx implementing indices rather than needing conversions to/from usize (which may even be costly, but this is unlikely).

cc @Zoxc as this'll both conflict but also help with #63756, at least from an audit perspective

r? @michaelwoerister @nikomatsakis

[Mark-Simulacrum]

@bors try @rust-timer queue

Let's gather some performance statistics here regardless of my beliefs that this shouldn't matter :)

[rust-timer]

Awaiting bors try build completion

[bors]

⌛ Trying commit c06df08 with merge 660d33983d348bd1162b1c3d8619df49a4c76706...

[Mark-Simulacrum]

I also have a follow-up PR that removes DepNode from DepNodeData -- but I think it could be applied onto master as well. That, I expect may have an effect here (at least on memory usage).

[Zoxc]

This does conflict with #61845 though. Your follow up PR sounds like it might be stepping on #62038 and #60035 too.

[rust-highfive]

The job mingw-check of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

2019-10-06T00:34:54.1805174Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-10-06T00:34:54.2006760Z ##[command]git config gc.auto 0
2019-10-06T00:34:54.2090056Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-10-06T00:34:54.2137451Z ##[command]git config --get-all http.proxy
2019-10-06T00:34:54.2275969Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/65146/merge:refs/remotes/pull/65146/merge
---
2019-10-06T00:44:57.4168824Z configure: build.locked-deps    := True
2019-10-06T00:44:57.4168873Z configure: llvm.ccache          := sccache
2019-10-06T00:44:57.4169122Z configure: build.cargo-native-static := True
2019-10-06T00:44:57.4169346Z configure: dist.missing-tools   := True
2019-10-06T00:44:57.4169607Z configure: build.configure-args := ['--enable-sccache', '--disable-manage-submodu ...
2019-10-06T00:44:57.4169742Z configure: writing `config.toml` in current directory
2019-10-06T00:44:57.4169789Z configure: 
2019-10-06T00:44:57.4170070Z configure: run `python /checkout/x.py --help`
2019-10-06T00:44:57.4170120Z configure: 
---
2019-10-06T00:49:01.7063060Z     Checking rustc_errors v0.0.0 (/checkout/src/librustc_errors)
2019-10-06T00:49:03.0770053Z     Checking fmt_macros v0.0.0 (/checkout/src/libfmt_macros)
2019-10-06T00:49:15.8445622Z     Checking syntax_ext v0.0.0 (/checkout/src/libsyntax_ext)
2019-10-06T00:49:24.9989295Z error[E0308]: mismatched types
2019-10-06T00:49:24.9990876Z     --> src/librustc/dep_graph/graph.rs:1095:57
2019-10-06T00:49:24.9991394Z      |
2019-10-06T00:49:24.9991902Z 1095 | ...                   let source = graph.data[source].node;
2019-10-06T00:49:24.9992942Z      |                                               |
2019-10-06T00:49:24.9992942Z      |                                               |
2019-10-06T00:49:24.9993497Z      |                                               expected reference, found struct `dep_graph::graph::DepNodeIndex`
2019-10-06T00:49:24.9994047Z      |                                               help: consider borrowing here: `&source`
2019-10-06T00:49:24.9995089Z      = note: expected type `&_`
2019-10-06T00:49:24.9995089Z      = note: expected type `&_`
2019-10-06T00:49:24.9995590Z                 found type `dep_graph::graph::DepNodeIndex`
2019-10-06T00:49:40.3652616Z error: aborting due to previous error
2019-10-06T00:49:40.3653746Z 
2019-10-06T00:49:40.3655356Z For more information about this error, try `rustc --explain E0308`.
2019-10-06T00:49:40.5573602Z error: could not compile `rustc`.
---
2019-10-06T00:49:40.5688336Z == clock drift check ==
2019-10-06T00:49:40.5721221Z   local time: Sun Oct  6 00:49:40 UTC 2019
2019-10-06T00:49:40.7214187Z   network time: Sun, 06 Oct 2019 00:49:40 GMT
2019-10-06T00:49:40.7214315Z == end clock drift check ==
2019-10-06T00:49:42.0062871Z ##[error]Bash exited with code '1'.
2019-10-06T00:49:42.0109674Z ##[section]Starting: Checkout
2019-10-06T00:49:42.0111316Z ==============================================================================
2019-10-06T00:49:42.0111363Z Task         : Get sources
2019-10-06T00:49:42.0111402Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

[bors]

☀️ Try build successful - checks-azure
Build commit: 660d33983d348bd1162b1c3d8619df49a4c76706 (660d33983d348bd1162b1c3d8619df49a4c76706)

[rust-timer]

Queued 660d33983d348bd1162b1c3d8619df49a4c76706 with parent 7870050, future comparison URL.

[rust-timer]

Finished benchmarking try commit 660d33983d348bd1162b1c3d8619df49a4c76706, comparison URL.

[Mark-Simulacrum]

Hm, so it's somewhat true that this conflicts with #61845 -- but I don't feel like it does so in a major way? That is, we likely then just want a ShardedIndexMap and making one doesn't seem too hard, right? It is added work, but that doesn't seem like an entirely bad thing to me, since it's mostly about introducing an otherwise helpful abstraction.

The main reason I thought it'd be good to this is because for #63756, this makes the lock movement to the interior much more obvious, rather than having to think of whether the change introduces lock ordering issues and such. Potentially also less helpful, though.

[michaelwoerister]

So, the change looks reasonable and straightforward but there is a noticeable slowdown on perf.rlo. I suspect that is because IndexMap is just a bit slower than hashbrown. Since the change also is in conflict (at least a bit) with other things going on in the same area, I'm inclined to not do them.

[Mark-Simulacrum]

Yeah, that's probably true. I agree that given the performance delta we should probably close.