There's also a bunch of the usual free-tier shenanigans on this host -- free instances with 1 user importing multiple large repositories -- but that's to be expected.

@epriestley The import has finished, but some commits show as importing still?

My impression is that there is a lot of room for optimization in the RepositoryPullLocalDaemon. I've wanted to tackle that for a while because it causes a fair delay between when a commit goes into git and when it actually shows up in Diffusion.

I can't remember off the top of my head but I'm pretty sure that there are some low hanging fruit in there.

Starting with repository update:

This repository uses pull requests, so it has about 3,200 refs and takes about 1.5s to list:

$ time git ls-remote https://github.com/concrete5/concrete5 | wc -l
3164
real	0m1.670s

In production, a git fetch origin '+refs/*:refs/*' --prune takes about 2.25s:

$ PHABRICATOR_INSTANCE=test-3uoifmrptebb time /core/lib/phabricator/bin/repository update --trace rC
...
>>> [29] <exec> $ git fetch origin '+refs/*:refs/*' --prune
<<< [29] <exec> 2,257,513 us

We also run git for-each-ref three times, at a cost of approximately 400ms each.

Internally, we use a commit cache to track repository heads. The size of this cache is 2048. This repository has 3,200 refs, so they fall out of the cache. The cache purges like a ring buffer (FIFO) so every ref triggers a cache miss.

The size of each cache entry is approximately 40 bytes (this is 40 "PHP Bytes", so probably a lot more than 40 real bytes, but likely less than 10x more), so we can freely use a much larger cache size.

During import, we poll repositories at the maximum rate (once every 15 seconds). This repository is also active enough to get maximum polling after import. Each update takes approximately 4s.

Here are some things we could do to improve this:

• Use git ls-remote and compare it to a local cache to detect whether we need to fetch or not. In theory, a no-op git fetch should have the same cost as git ls-remote, but it appears that it does not.
• Identify callers to git for-each-ref and have them use a cache.
• Increase the size of the commit cache.
• When bulk-filling the commit cache, refuse to fill more entries than the size of the cache.
• While a repository is importing, we could do a reduced-cost update which just checks for import progress but does not perform a fetch. It's unclear if this is worthwhile to pursue.

Of these, I believe only the cache changes are straightforward, and they are not an overwhelming fraction of the runtime.

In T12296#212850, @Mnkras wrote:

@epriestley The import has finished, but some commits show as importing still?

I can't reproduce this, perhaps it resolved itself?

$ PHABRICATOR_INSTANCE=test-3uoifmrptebb /core/lib/phabricator/bin/repository importing R1 R2
No importing commits found.

@epriestley
https://test-3uoifmrptebb.phacility.com/R2:80233a27d7b72bb03710d358e12a531ce46808a9

Shows as importing in the ui

Ah, okay. This is the relevant part:

This commit has been deleted in the repository: it is no longer reachable from any branch, tag, or ref.

We stop importing commits if we detect that they have become unreachable, which may strand commits in a partially imported state. The UI could be more clear about this.

The commit cache should be a fairly large performance win.

@epriestley all other commits that have the message "This commit has been deleted in the repository: it is no longer reachable from any branch, tag, or ref." have imported, so I don't think the reachable code is working correctly, as until within the last few weeks I wasn't seeing these "This commit has been deleted in the repository: it is no longer reachable from any branch, tag, or ref." commits showing up, but now I see them all the time.

It is expected that some deleted commits will import and others will not.

If a commit imports before it is deleted, it will import. For example: push a branch, wait for the commits on it to import, delete the branch.

If a commit does not import before it is deleted, the import will abort. For example: push a branch, don't wait for the commits on it to import, delete the branch.

Can you explain in more detail why you think the code isn't working correctly? For example, what is the hash of a commit which you believe to be reachable but which you think is incorrectly marked as unreachable, or vice versa?

The reason I think something isn't working correctly is in my dashboard, I list recent commits, and since I updated my install a few weeks ago my commit list looks like this a lot:

Previously, I never saw these deleted commits show up, and they tend to be pretty useless/duplicates

If you don't want to see unreachable commits, use a query which hides them:

That works, Im curious when this changed, as I didn't need to filter since the last few weeks.

Likely somewhere in T9430 / T10978, which rewrote a big chunk of the audit code.

(That said, my expectation is that behaviors around unreachable commits have not changed -- D17279 added the "Unreachable" option to solve this problem.)

Cool, now I don't feel so bad since I know you only recently added the option 😄

Thanks!

While the git ls-remote change isn't really motivated as a performance improvement, it does seem to have reduced CPU usage a measurable amount (deployed on the morning of 3/18), maybe 15%:

There's no appreciable change in disk I/O or network I/O that I can see from the graphs.

In T12296#216132, @epriestley wrote:

While the git ls-remote change isn't really motivated as a performance improvement, it does seem to have reduced CPU usage a measurable amount (deployed on the morning of 3/18), maybe 15%:

I am seeing a similar drop in CPU use on wikimedia's phabricator host, it looks closer to 10% but still significant.