Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: change default log revset to not include all tagged heads #3095

Merged
merged 1 commit into from
Feb 22, 2024

Conversation

yuja
Copy link
Contributor

@yuja yuja commented Feb 20, 2024

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

@yuja yuja force-pushed the push-ussuprqzkuvx branch from 07578cb to ec683a0 Compare February 20, 2024 06:56
The default immutable_heads() includes tags(), which makes sense, but computing
heads(tags()) can be expensive because the tags() set is usually sparse. For
example, "jj bench revset 'heads(tags())'" took 157ms in my linux stable
mirror. We can of course optimize the heads evaluation by using bit set or
segmented index, but the query includes many historical heads if the repository
has per-release branches, which are uninteresting anyway. So, this patch
replaces heads(immutable_heads()) with trunk().

The reason we include heads(immutable_heads()) is to mitigate the following
problem. Suppose trunk() is the branch to be based off, I think using trunk()
here is pretty good.

```
A   B
*---*----* trunk() ⊆ immutable_heads()
     \
      * C
```
jj-vcs#2247 (comment)
@yuja yuja force-pushed the push-ussuprqzkuvx branch from ec683a0 to 5b7ecb8 Compare February 20, 2024 12:16
@yuja yuja merged commit 62f0cb8 into jj-vcs:main Feb 22, 2024
15 checks passed
@yuja yuja deleted the push-ussuprqzkuvx branch February 22, 2024 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants