Make `setup_dep_graph` incremental-only and more straightforward
The existing code contains some very strange control flow that can put the dep graph into an inconsistent and untested state if streaming output setup fails.
(Specifically, that failure would put `DepGraph` into an "empty" state intended for non-incremental compilation, but other parts of the compiler would still think that incremental mode is enabled due to `sess.opts.incremental`.)
This PR therefore performs a big overhaul of `setup_dep_graph` by:
- Returning immediately in non-incremental mode.
- Exiting immediately if dep-graph streaming output couldn't be set up.
- Inlining some "helper" functions that were more confusing than helpful.
Make typeck a tcx method which calls typeck_root query
Currently typeck query itself calls `tcx.typeck(tcx.typeck_root_def_id(key))` if its key isn't a type-check root. I thought this might be an overhead and made typeck a tcx method which calls typeck_root query instead.
This is a step to simplify `cache_on_disk_if` query modifier.
@petrochenkov please run perf
The existing code contains some very strange control flow that can put the dep
graph into an inconsistent and untested state if streaming output setup fails.
(Specifically, that failure would put `DepGraph` into an "empty" state intended
for non-incremental compilation, but other parts of the compiler would still
think that incremental mode is enabled due to `sess.opts.incremental`.)
This commit therefore performs a big overhaul of `setup_dep_graph` by:
- Returning immediately in non-incremental mode.
- Exiting immediately if dep-graph streaming output couldn't be set up.
- Inlining some "helper" functions that were more confusing than helpful.
The standard library has helper methods for converting integers to/from
little-endian bytes, for checked conversion from `usize` to `u8`, and for
treating `&mut T` as `&mut [T; 1]`.
Returning dedicated structs and enums makes the meaning of each return value
more obvious, and provides a more natural home for documentation.
The intermediate `load_data` function was unhelpful, and has been inlined into
the main function.
Every other part of `OnDiskCache` deals with loading information from the
_previous_ session, except for this one field.
Moving it out to `QuerySystem` makes more sense, because that's also where
query return values are stored (inside the caches in their vtables).
Remove the confusing word "error". The diagnostic is already prefixed
with a level when it is displayed, so this is redundant and possibly
confusing ("warning: error ...").
Add some help text summarizing the impact of what happened: the next
build won't be able to reuse work from the current run.
The warning has no error code, so in a `-D warnings` environment, it's
impossible to ignore if it consistently breaks your build. Change it
to a note so it is still visible, but doesn't break the build
Main changes:
- Inline `encode_query_cache` and `TyCtxt::serialize_query_result_cache`
- Pull value promotion out of `OnDiskCache::drop_serialized_data`
- Panic if `on_disk_cache` is None in an incremental-only path
Rename `DepGraphQuery` to `RetainedDepGraph`
This is a revised subset of https://github.com/rust-lang/rust/pull/152836 that only performs an internal renaming, and does not touch the `-Zquery-dep-graph` flag.
The new name and comments for `RetainedDepGraph` should hopefully do a better job of communicating that it is not used in normal compiler operation, even in incremental mode.
It was removed in #115920 to enable it being moved to
`rustc_query_system`, a move that has recently been reversed. It's much
simpler as an enum.
Also:
- Remove the overly complicated `Debug` impl for `DepKind`.
- Remove the trivial `DepKind` associated constants (`NULL` et al.)
- Add an assertion to ensure that the number of `DepKinds` fits within a
`u16`.
- Rename `DEP_KIND_VARIANTS` as `DEP_KIND_NUM_VARIANTS`, to make it
clearer that it's a count, not a collection.
- Use `stringify!` in one place to make the code clearer.
By removing the generic `D` parameter from `DepGraph`, `DepGraphData`,
`CurrentDepGraph`, `SerializedDepGraph`, `SerializedNodeHeader`, and
`EncoderState`.
In `setup_dep_graph`, we set up a session directory for the current
incremental compilation session, load the dep graph, and then GC stale
incremental compilation sessions for the crate. The freshly-created
session directory ends up in this list of potentially-GC'd directories
but in practice is not typically even considered for GC because the new
directory is neither finalized nor `is_old_enough_to_be_collected`.
Unfortunately, `is_old_enough_to_be_collected` is a simple time check,
and if `load_dep_graph` is slow enough it's possible for the
freshly-created session directory to be tens of seconds old already.
Then, old enough to be *eligible* to GC, we try to `flock::Lock` it as
proof it is not owned by anyone else, and so is a stale working
directory.
Because we hold the lock in the same process, the behavior of
`flock::Lock` is dependent on platform-specifics about file locking
APIs. `fcntl(F_SETLK)`-style locks used on non-Linux Unices do not
provide mutual exclusion internal to a process. `fcntl_locking(2)` on
Linux describes some relevant problems:
```
The record locks described above are associated with the process
(unlike the open file description locks described below). This
has some unfortunate consequences:
* If a process closes any file descriptor referring to a file,
then all of the process's locks on that file are released, [...]
* The threads in a process share locks. In other words, a
multithreaded program can't use record locking to ensure that
threads don't simultaneously access the same region of a file.
```
`fcntl`-locks will appear to succeed to lock the fresh incremental
compilation directory, at which point we can remove it just before using
it later for incremental compilation. Saving incremental compilation
state later fails and takes rustc with it with an error like
```
[..]/target/debug/incremental/crate-<hash>/<name>/dep-graph.part.bin: No such file or directory (os error 2)
```
The release-lock-on-close behavior has uncomfortable consequences for
the freshly-opened file description for the lock, but I think in
practice isn't an issue. If we would close the file, we failed to
acquire the lock, so someone else had the lock ad we're not releasing
locks prematurely.
`flock(LOCK_EX)` doesn't seem to have these same issues, and because
`flock::Lock::new` always opens a new file description when locking, I
don't think Linux can have this issue.
From reading `LockFileEx` on MSDN I *think* Windows has locking
semantics similar to `flock`, but I haven't tested there at all.
My conclusion is that there is no way to write a pure-POSIX
`flock::Lock::new` which guarantees mutual exclusion across different
file descriptions of the same file in the same process, and
`flock::Lock::new` must not be used for that purpose. So, instead, avoid
considering the current incremental session directory for GC in the
first place. Our own `sess` is evidence we're alive and using it.
This was done in #145740 and #145947. It is causing problems for people
using r-a on anything that uses the rustc-dev rustup package, e.g. Miri,
clippy.
This repository has lots of submodules and subtrees and various
different projects are carved out of pieces of it. It seems like
`[workspace.dependencies]` will just be more trouble than it's worth.
Use thread local dep graph encoding
This adds thread local encoding of dep graph nodes. Each thread has a `MemEncoder` that gets flushed to the global `FileEncoder` when it exceeds 64 kB. Each thread also has a local cache of dep indices. This means there can now be empty gaps in `SerializedDepGraph`.
Indices are marked green and also allocated by the new atomic operation `DepNodeColorMap::try_mark_green` as the encoder lock is removed.
I'm removing empty identifiers everywhere, because in practice they
always mean "no identifier" rather than "empty identifier". (An empty
identifier is impossible.) It's better to use `Option` to mean "no
identifier" because you then can't forget about the "no identifier"
possibility.
Some specifics:
- When testing an attribute for a single name, the commit uses the
`has_name` method.
- When testing an attribute for multiple names, the commit uses the new
`has_any_name` method.
- When using `match` on an attribute, the match arms now have `Some` on
them.
In the tests, we now avoid printing empty identifiers by not printing
the identifier in the `error:` line at all, instead letting the carets
point out the problem.
Avoid no-op unlink+link dances in incr comp
Incremental compilation scales quite poorly with the number of CGUs. This PR improves one reason for that.
The incr comp process hard-links all the files from an old session into a new one, then it runs the backend, which may just hard-link the new session files into the output directory. Then codegen hard-links all the output files back to the new session directory.
This PR (perhaps unimaginatively) fixes the silliness that ensues in the last step. The old `link_or_copy` implementation would be passed pairs of paths which are already the same inode, then it would blindly delete the destination and re-create the hard-link that it just deleted. This PR lets us skip both those operations. We don't skip the other two hard-links.
`cargo +stage1 b && touch crates/core/main.rs && strace -cfw -elink,linkat,unlink,unlinkat cargo +stage1 b` before and then after on `ripgrep-13.0.0`:
```
% time seconds usecs/call calls errors syscall
------ ----------- ----------- --------- --------- ----------------
52.56 0.024950 25 978 485 unlink
34.38 0.016318 22 727 linkat
13.06 0.006200 24 249 unlinkat
------ ----------- ----------- --------- --------- ----------------
100.00 0.047467 24 1954 485 total
```
```
% time seconds usecs/call calls errors syscall
------ ----------- ----------- --------- --------- ----------------
42.83 0.014521 57 252 unlink
38.41 0.013021 26 486 linkat
18.77 0.006362 25 249 unlinkat
------ ----------- ----------- --------- --------- ----------------
100.00 0.033904 34 987 total
```
This reduces the number of hard-links that are causing perf troubles, noted in https://github.com/rust-lang/rust/issues/64291 and https://github.com/rust-lang/rust/issues/137560
Stuart Cook