Almost the entire method deals with `FileType::Normal`, so we can
reduce indentation and repeated matching on the file type by doing it
early and returning in the non-normal-file cases.
For tree-level conflicts, we're eventually not going to have
`ConflictId`. We'd want to make `write_conflict_to_store()` take a
`Merge<Option<TreeValue>>` and return an updated such value. That
would leave very little logic in the function, so let's just inline it
instead.
`update_from_content()` already writes file content for each term of
an unresolved merge, so it seems consistent for it to also write the
file content for resolved merges. I think this should simplify further
refactoring for tree-level conflicts and for preserving the executable
bit.
Since `update_from_contents()` only works with file contents and not
the executable or other kinds of paths, I think it makes more sense
for it to deal with `FileId`s instead of `TreeValue`s.
There were still many instances of `conflict` left from before we
renamed `Conflict<T>` to `Merge<T>`. I decided to rename many of them
based on the type parameter instead of the container. I think that
made it more readable in many cases.
I think I moved way too many functions onto `Merge<Option<TreeValue>>`
in 82883e648d. This effectively reverts almost all of that
commit. The `Merge<T>` type is simple container and it seems like it
should be at fairly low level in the dependency graph. By moving
functions off of it, we can get rid of the back-depdencies from the
`merge` module to the `conflict` module that I introduced when I moved
`Merge` to the `merge` module. I'm thinking the `conflict` module can
focus on materialized conflicts.
Per discussion in #2009. This behavior isn't affected by e7e49527ef "git:
ensure that remote branches never diverge", but it's subtle enough to write
a test.
I was considering how refs would be imported if we had a per-remote view of
named branches (and tags): Each remote has a view, and jj remembers the last
known view state to compute diffs. That's the same for the pseudo "git" remote.
Under the current storage, these view states are represented as follows:
git_refs["refs/heads/{name}"] # pseudo "git" remote branches
git_refs["refs/tags/{name}"] # pseudo "git" remote tags
git_refs["refs/remotes/{remote}/{name}"] # real remote branches
and the diffs are merged in to branches[name].local_target and tags[name].
We also have branches[name].remote_targets[remote], but I think it's redundant
because a tracking branch should also be the last known state, not something
that can diverge from the actual state. To make that clear, this commit
replaces the use of the "merge" API.
As reported in #1970, SSH authentication would sometimes run into a
loop where it repeatedly tries to use ssh-agent for authentication
without making progess. The problem can be reproduced by simply
removing `$SSH_AUTH_KEY` from your environment (and not having a Git
credentials helper configured, I think).
This seems to be a bug introduced by b104f8e154c21. That commit meant
to make it so we attempt to use ssh-agent and fall back to using
(password-less) keys after that. The problem is that
`git2::Cred::ssh_key_from_agent()` just returns an object that will be
used later for looking up the credentials from ssh-agent, so the call
will not fail because ssh-agent is not reachable.
This commit attempts to fix the problem by having the credentials
callback attempt to use ssh-agent only once.
Perhaps the most important invariant in `.jj/working_copy/tree_state`
is that its set of files in it matches the files in its tree. In
particular, if a file that exists in the tree doesn't exist in the
file state and doesn't exist on disk either, we won't notice that it's
gone, and we will therefore not delete it from the tree on future
rounds of snapshotting either.
Now that we process the outputs from the file system traversal by
reading from channels, we can separate the processing from the file
system traversal. When the working copy is unchanged, processing tree
entries and deleted files takes practically no time, but processing
file states and present files takes significant time.
Since `Conflict<T>` can also represent a non-conflict state (a single
term), `Merge<T>` seems like better name.
Thanks to @ilyagr for the suggestion in
https://github.com/martinvonz/jj/pull/1774#discussion_r1257547709
Sorry about the churn. It would have been better if I thought of this
name before I introduced `Conflict<T>`.
Summary: There's no need to go around specifying `rust-version` or `edition` or
`version` several times, now that we have a global workspace. Instead, inherit
workspace metadata from the top-level Cargo.toml file.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Change-Id: Iaf905445978ed2b3377239dcdb8a6c32
Summary: This moves all dependencies across the jj-lib and jj-cli crates into
the top-level Cargo file; with that, we can change each crate instead to just
inherit the workspace version, with the toggled features enabled, by setting
a dependency such as:
dep.workspace = true
in the relevant Cargo.toml file.
This doesn't actually change any of the build semantics (from what I can tell)
nor the lockfile, and seems to respond normally. There are more cleanups that
can follow.
Two notes:
- Dependabot seems to work fine, based on what I've seen in other repos.
- `cargo add` doesn't seem to know how to add packages to a top-level
`workspace.dependencies` field; instead you can `cargo add -p jj-cli`
and move the entries, at least.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Change-Id: I307827e5f15c0d8ea8e2a80ec793d3c7
This is simpler than carefully tracking mutation through old/new git refs and
merged local branches. There are two subtle behavior changes:
a. unimported git refs excluded by git_ref_filter() are not pinned.
b. unexported branches are pinned (so fetched deletion doesn't abandon the
branch if it's referenced by another branch.)
I think (a) is okay (and even more correct) since such refs aren't known to jj
yet. (b) is desired.
This appears to be a bit slower (1.170s -> 1.211s with "log -R git -r 'tags()'
-Tcommit_id --ignore-working-copy"), but seemed better than keeping growing
cache.
This improves `jj status` time by a factor of ~2x on my machine (M1 Macbook Pro 2021 16-inch, uses an SSD):
```sh
$ hyperfine --parameter-list hash before,after --parameter-list repo nixpkgs,gecko-dev --setup 'git checkout {hash} && cargo build --profile release-with-debug' --warmup 3 './target/release-with-debug/jj -R ../{repo} st'
Benchmark 1: ./target/release-with-debug/jj -R ../nixpkgs st (hash = before)
Time (mean ± σ): 1.640 s ± 0.019 s [User: 0.580 s, System: 1.044 s]
Range (min … max): 1.621 s … 1.673 s 10 runs
Benchmark 2: ./target/release-with-debug/jj -R ../nixpkgs st (hash = after)
Time (mean ± σ): 760.0 ms ± 5.4 ms [User: 812.9 ms, System: 2214.6 ms]
Range (min … max): 751.4 ms … 768.7 ms 10 runs
Benchmark 3: ./target/release-with-debug/jj -R ../gecko-dev st (hash = before)
Time (mean ± σ): 11.403 s ± 0.648 s [User: 4.546 s, System: 5.932 s]
Range (min … max): 10.553 s … 12.718 s 10 runs
Benchmark 4: ./target/release-with-debug/jj -R ../gecko-dev st (hash = after)
Time (mean ± σ): 5.974 s ± 0.028 s [User: 5.387 s, System: 11.959 s]
Range (min … max): 5.937 s … 6.024 s 10 runs
$ hyperfine --parameter-list repo nixpkgs,gecko-dev --warmup 3 'git -C ../{repo} status'
Benchmark 1: git -C ../nixpkgs status
Time (mean ± σ): 865.4 ms ± 8.4 ms [User: 119.4 ms, System: 1401.2 ms]
Range (min … max): 852.8 ms … 879.1 ms 10 runs
Benchmark 2: git -C ../gecko-dev status
Time (mean ± σ): 2.892 s ± 0.029 s [User: 0.458 s, System: 14.244 s]
Range (min … max): 2.837 s … 2.934 s 10 runs
```
Conclusions:
- ~2x improvement from previous `jj status` time.
- Slightly faster than Git on nixpkgs.
- Still 2x slower than Git on gecko-dev, not sure why.
For reference, Git's default number of threads is defined in the `online_cpus` function: ee48e70a82/thread-utils.c (L21-L66). We are using whatever the Rayon default is.
In preparation of traversing the filesystem in parallel, send updates via `channel`.
An alternative is to modify shared mutable state, e.g. put `self.file_states` behind a mutex or use a concurrent hash-map. This risks leaving the `TreeState` in an invalid state if an error occurs, and makes invariants harder to reason about.
Using a channel introduces a small performance regression. (I didn't try out the concurrent hash-map approach.)
```sh
$ hyperfine --parameter-list hash before,after --setup 'git checkout {hash} && cargo build --profile release-with-debug' --warmup 3 './target/release-with-debug/jj -R ../nixpkgs st'
Benchmark 1: ./target/release-with-debug/jj -R ../nixpkgs st (hash = before)
Time (mean ± σ): 1.533 s ± 0.013 s [User: 0.587 s, System: 0.926 s]
Range (min … max): 1.510 s … 1.559 s 10 runs
Benchmark 2: ./target/release-with-debug/jj -R ../nixpkgs st (hash = after)
Time (mean ± σ): 1.563 s ± 0.021 s [User: 0.607 s, System: 0.936 s]
Range (min … max): 1.518 s … 1.595 s 10 runs
Summary
./target/release-with-debug/jj -R ../nixpkgs st (hash = before) ran
1.02 ± 0.02 times faster than ./target/release-with-debug/jj -R ../nixpkgs st (hash = after)
```
