Skip to content

working_copy: avoid "empty" fsmonitor saves - #10049

Open
jstasiak wants to merge 1 commit into
jj-vcs:mainfrom
jstasiak:optimize-fsmonitor-empty-saves
Open

working_copy: avoid "empty" fsmonitor saves#10049
jstasiak wants to merge 1 commit into
jj-vcs:mainfrom
jstasiak:optimize-fsmonitor-empty-saves

Conversation

@jstasiak

@jstasiak jstasiak commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

I've been looking at jj's performance in a large (for me) repository: Firefox.

It only became bearable with fsmonitor/watchman but I discovered that even with no files changing advancing the fsmonitor clock triggered the complete tree state rewrite.

I figured we could just keep the old clock if there are no changes.

Here are some "jj st" numbers from my system (macOS 26.5.2, M1 Pro,
fast NVMe, hardcoded WATCHMAN_SOCK to avoid watchman socket discovery):

jj version                Runs  Median ms  Mean ms   Min ms   Max ms
stable 0.44.0             20    210.990    257.952   190.111  836.981
7190eda8 + this patch     20    135.723    136.843   127.764  153.652

I expect this to be safe as querying again from the older clock can replay paths, but it cannot omit future file updates.

Stable minus custom is 75.267 ms median, 121.109 ms mean, 62.347 ms minimum, and 683.329 ms maximum.

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/updated tests to cover my changes
  • I fully understand the code that I am submitting (what it does,
    how it works, how it's organized), including any code drafted by an LLM.
  • For any prose generated by an LLM, I have proof-read and copy-edited with
    an eye towards deleting anything that is irrelevant, clarifying anything
    that is confusing, and adding details that are relevant. This includes,
    for example, commit descriptions, PR descriptions, and code comments.

@jstasiak
jstasiak requested a review from a team as a code owner August 24, 2026 21:30
I've been looking at jj's performance in a large (for me) repository:
Firefox.

It only became bearable with fsmonitor/watchman but I discovered that
even with no files changing advancing the fsmonitor clock triggered the
complete tree state rewrite.

I figured we could just keep the old clock if there are no changes.

Here are some "jj st" numbers from my system (macOS 26.5.2, M1 Pro,
fast NVMe, hardcoded WATCHMAN_SOCK to avoid watchman socket discovery):

    jj version                Runs  Median ms  Mean ms   Min ms   Max ms
    stable 0.44.0             20    210.990    257.952   190.111  836.981
    7190eda + this patch     20    135.723    136.843   127.764  153.652

I expect this to be safe as querying again from the older clock can
replay paths, but it cannot omit future file updates.

Stable minus custom is 75.267 ms median, 121.109 ms mean, 62.347 ms
minimum, and 683.329 ms maximum.
@jstasiak
jstasiak force-pushed the optimize-fsmonitor-empty-saves branch from a4d3eb4 to 9150716 Compare August 24, 2026 21:37

@yuja yuja left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@arxanas Any concern about not updating the watchman clock?

// old clock after that scan even if it found untracked paths.
if should_update_watchman_clock
&& ((stats.untracked_paths.is_empty() && stats.invalid_utf8_paths.is_empty())
|| watchman_clock.is_none())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: It seems slightly easier to follow nested ifs rather than combining multiple && and || conditions.

// Reusing the older clock may make a later query do more work, but it
// cannot omit changes.
let should_update_watchman_clock =
!matches!(&changed_files, Some(files) if files.is_empty());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: .as_ref().is_some_and() or .is_none_or() also works.

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