Address review: rename field, lazy-build helper, fold decode loops

Rename the reverse-index field from `index` to `reverse_index`. Move the
per-kind fingerprint map build into `LazyKindIndex::fingerprint_map` and
have it `debug_assert` each node's kind matches the kind being built, so
`node_to_index_opt` is just the lookup. Read the per-kind counts and build
their offset ranges in a single decode loop, and store the grouped indices
as `Option<SerializedDepNodeIndex>` (sized by `node_count`) so an unfilled
slot is `None` rather than a real-looking index; the counting sort fills
every slot of each range exactly once. Document why `Null` nodes are
skipped.

Author: xmakro

compiler/rustc_middle/src/dep_graph/serialized.rs

@@ -51,7 +51,7 @@ use rustc_data_structures::outline;
 use rustc_data_structures::profiling::SelfProfilerRef;
 use rustc_data_structures::sync::{AtomicU64, Lock, WorkerLocal, broadcast};
 use rustc_data_structures::unhash::UnhashMap;
-use rustc_index::IndexVec;
+use rustc_index::{IndexSlice, IndexVec};
 use rustc_serialize::opaque::mem_encoder::MemEncoder;
 use rustc_serialize::opaque::{FileEncodeResult, FileEncoder, IntEncodedWithFixedSize, MemDecoder};
 use rustc_serialize::{Decodable, Decoder, Encodable, Encoder};
@@ -116,7 +116,7 @@ pub struct SerializedDepGraph {
     /// The lazily-built inverse of `nodes`: maps a [`DepNode`] back to its
     /// [`SerializedDepNodeIndex`] via the node's key fingerprint. See
     /// [`LazyNodeIndex`].
-    index: LazyNodeIndex,
+    reverse_index: LazyNodeIndex,
     /// The number of previous compilation sessions. This is used to generate
     /// unique anon dep nodes per session.
     session_count: u64,
@@ -140,8 +140,11 @@ pub struct SerializedDepGraph {
 #[derive(Debug, Default)]
 struct LazyNodeIndex {
     /// All (non-`Null`) node indices, grouped into contiguous per-`DepKind`
-    /// ranges described by `kinds`.
-    nodes_by_kind: Vec<SerializedDepNodeIndex>,
+    /// ranges described by `kinds`. Allocated with `None` in every slot; the
+    /// counting sort in `decode` fills each slot of every range exactly once, so a
+    /// `None` is never read back. `SerializedDepNodeIndex` has a niche, so the
+    /// `Option` does not grow the vector.
+    nodes_by_kind: Vec<Option<SerializedDepNodeIndex>>,
     /// For each `DepKind`, the range of `nodes_by_kind` holding its node indices
     /// and the lazily-built fingerprint map over that range.
     kinds: Vec<LazyKindIndex>,
@@ -158,6 +161,43 @@ struct LazyKindIndex {
     map: OnceLock<UnhashMap<PackedFingerprint, SerializedDepNodeIndex>>,
 }
 
+impl LazyKindIndex {
+    /// Returns this kind's `key_fingerprint -> node index` map, building it from
+    /// the kind's range of `nodes_by_kind` on the first call. Building does the
+    /// per-node hash-map inserts that decode used to perform eagerly for every
+    /// kind; here we only pay it for kinds that are actually looked up.
+    fn fingerprint_map(
+        &self,
+        kind: DepKind,
+        nodes: &IndexSlice<SerializedDepNodeIndex, DepNode>,
+        nodes_by_kind: &[Option<SerializedDepNodeIndex>],
+    ) -> &UnhashMap<PackedFingerprint, SerializedDepNodeIndex> {
+        self.map.get_or_init(|| {
+            let range = (self.start as usize)..(self.start as usize + self.len as usize);
+            let mut map =
+                UnhashMap::with_capacity_and_hasher(self.len as usize, Default::default());
+            for &idx in &nodes_by_kind[range] {
+                let idx = idx.expect("counting sort fills every slot of a kind's range");
+                let node = &nodes[idx];
+                debug_assert_eq!(node.kind, kind);
+                if map.insert(node.key_fingerprint, idx).is_some()
+                    // Side effect nodes can legitimately share a fingerprint.
+                    && node.kind != DepKind::SideEffect
+                {
+                    panic!(
+                        "Error: A dep graph node ({kind:?}) does not have an unique index. \
+                         Running a clean build on a nightly compiler with \
+                         `-Z incremental-verify-ich` can help narrow down the issue for reporting. \
+                         A clean build may also work around the issue.\n
+                         DepNode: {node:?}"
+                    )
+                }
+            }
+            map
+        })
+    }
+}
+
 impl SerializedDepGraph {
     #[inline]
     pub fn edge_targets_from(
@@ -188,28 +228,9 @@ impl SerializedDepGraph {
 
     #[inline]
     pub fn node_to_index_opt(&self, dep_node: &DepNode) -> Option<SerializedDepNodeIndex> {
-        let kind = self.index.kinds.get(dep_node.kind.as_usize())?;
-        let map = kind.map.get_or_init(|| {
-            let range = (kind.start as usize)..(kind.start as usize + kind.len as usize);
-            let mut map = UnhashMap::with_capacity_and_hasher(kind.len as usize, Default::default());
-            for &idx in &self.index.nodes_by_kind[range] {
-                let node = &self.nodes[idx];
-                if map.insert(node.key_fingerprint, idx).is_some()
-                    // Side effect nodes can legitimately share a fingerprint.
-                    && node.kind != DepKind::SideEffect
-                {
-                    panic!(
-                        "Error: A dep graph node ({:?}) does not have an unique index. \
-                         Running a clean build on a nightly compiler with \
-                         `-Z incremental-verify-ich` can help narrow down the issue for reporting. \
-                         A clean build may also work around the issue.\n
-                         DepNode: {node:?}",
-                        node.kind
-                    )
-                }
-            }
-            map
-        });
+        let kind = self.reverse_index.kinds.get(dep_node.kind.as_usize())?;
+        let map =
+            kind.fingerprint_map(dep_node.kind, &self.nodes, &self.reverse_index.nodes_by_kind);
         map.get(&dep_node.key_fingerprint).copied()
     }
 
@@ -345,43 +366,49 @@ impl SerializedDepGraph {
         // end of the array. This padding ensure it doesn't.
         edge_list_data.extend(&[0u8; DEP_NODE_PAD]);
 
-        // Read the number of nodes of each dep kind. These per-kind counts let us
-        // group node indices by kind below; the fingerprint -> index maps
-        // themselves are built lazily, per kind, on first lookup (see
-        // `LazyNodeIndex`).
-        let kind_counts: Vec<u32> = (0..(DepKind::MAX + 1)).map(|_| d.read_u32()).collect();
-
-        let session_count = d.read_u64();
-
-        // Counting sort: place every (non-`Null`) node index into a contiguous
-        // per-kind range of `nodes_by_kind`, using the per-kind counts to compute
-        // each kind's start offset. Padding gaps (`DepKind::Null`) are never
-        // looked up by fingerprint, so we skip them.
-        let mut kinds = Vec::with_capacity(kind_counts.len());
+        // Read the number of nodes of each dep kind, turning each per-kind count
+        // into a contiguous, still-empty range of `nodes_by_kind` as we go (a
+        // prefix sum of the counts gives each kind's start offset). The
+        // fingerprint -> index map for a kind is built lazily, on the first lookup
+        // of that kind (see `LazyKindIndex::fingerprint_map`).
+        let mut kinds = Vec::with_capacity(DepKind::MAX as usize + 1);
         let mut offset = 0u32;
-        for &len in &kind_counts {
+        for _ in 0..(DepKind::MAX + 1) {
+            let len = d.read_u32();
             kinds.push(LazyKindIndex { start: offset, len, map: OnceLock::new() });
             offset += len;
         }
+        // Every encoded node has a (non-`Null`) kind, so the per-kind counts sum to
+        // `node_count`; `Null` nodes are never encoded, so no range is reserved for
+        // them.
         debug_assert_eq!(offset as usize, node_count);
-        let mut nodes_by_kind = vec![SerializedDepNodeIndex::from_u32(0); offset as usize];
+
+        let session_count = d.read_u64();
+
+        // Counting sort: place each node index into its kind's range. `fill[k]`
+        // points at the next free slot in kind `k`'s range, so a kind's nodes end
+        // up contiguous. Slots start as `None` and are each filled exactly once
+        // (the counts sum to the number of non-`Null` nodes).
+        let mut nodes_by_kind = vec![None; node_count];
         let mut fill: Vec<u32> = kinds.iter().map(|k| k.start).collect();
         for (idx, node) in nodes.iter_enumerated() {
+            // Unused indices from batch allocation stay `Null`; they carry no
+            // encoded node and are never looked up by fingerprint, so skip them.
             if node.kind == DepKind::Null {
                 continue;
             }
             let k = node.kind.as_usize();
-            nodes_by_kind[fill[k] as usize] = idx;
+            nodes_by_kind[fill[k] as usize] = Some(idx);
             fill[k] += 1;
         }
-        let index = LazyNodeIndex { nodes_by_kind, kinds };
+        let reverse_index = LazyNodeIndex { nodes_by_kind, kinds };
 
         Arc::new(SerializedDepGraph {
             nodes,
             value_fingerprints,
             edge_list_indices,
             edge_list_data,
-            index,
+            reverse_index,
             session_count,
         })
     }