# Parallel Compiler Audit: Fine grained locks in DepGraph

###### tags: `parallel-compiler`

## Conclusions

- would be good to extract CurrentDepGraph to separate module for auditability
- not doing that yet due to future work in this area

PR: https://github.com/rust-lang/rust/pull/63756

- `git remote add Zoxc [e-mail]:Zoxc/rust.git`
- `git fetch Zoxc sharded-dep-graph-1`
- `git checkout sharded-dep-graph-1`

## Current situation

- Single lock around CurrentDepGraph
- No internal locks in CurrentDepGraph
- There are no concrete docs around requirements between the fields.
- Lock is not consistently held for function bodies
- doesn't seem to be re-acquired, either, but hard to audit
- did not look at issues, but no recollection of problems with this lock

Fields:

- `anon_id_seed` is constant
- `forbidden_edge` is constant

- `data: IndexVec<DepNodeIndex, DepNodeData>`
and `node_to_node_index: FxHashMap<DepNode, DepNodeIndex>`
- maps are kept in sync, such that DepNode -> DepNodeIndex -> DepNodeData always holds
- unclear why two maps
- we may want a `IndexMap<DepNode, DepNodeData>`.
- DepNodeIndex is easily obtainable
- O(1) lookup
- https://github.com/rust-lang/rust/pull/65146

- `total_read_count`, `total_duplicate_read_count`
* only touched in debug_assertions mode (though not enforced)
* [only increased](https://github.com/rust-lang/rust/blob/7870050796e5904a0fc85ecbe6fa6dde1cfe0c91/src/librustc/dep_graph/graph.rs#L1090-L1110)
* [read in one place](https://github.com/rust-lang/rust/blob/7870050796e5904a0fc85ecbe6fa6dde1cfe0c91/src/librustc/dep_graph/graph.rs#L497-L498)
* for debug printing during encode_dep_graph
* presumably one-off, after all changes

## PR proposal

https://github.com/Zoxc/rust/blob/sharded-dep-graph-1/src/librustc/dep_graph/graph.rs

- Lock removed around CurrentDepGraph

Concerns:

- CurrentDepGraph is not in its own module. Fields are as such public outside its methods
- Atomic ordering for total_read_count and total_duplicate_read_count

Changes:

* Individual locks around `data` and `node_to_node_index` fields
* lock ordering is `node_to_node_index`, then `data`.
* both only acquired when adding new nodes
* no known problems, but also does not seem helpful
* followup PRs here probably bring most of weight?

* total_read_count, total_duplicate_read_count
* moved to AtomicU64
* Uses Acquire ordering (why?) on read, SeqCst on increment
* inconsistent
* may want separate meeting to discuss
* Alex has noted preference for SeqCst everywhere