Page MenuHomePhabricator

Speed up diffusion_browsequery for mercurial repositories
ClosedPublic

Authored by richardvanvelzen on Feb 7 2014, 11:44 AM.
Tags
None
Referenced Files
F14379749: D8163.id18469.diff
Sat, Dec 21, 3:11 AM
F14376751: D8163.diff
Sat, Dec 21, 12:15 AM
Unknown Object (File)
Wed, Dec 11, 4:50 PM
Unknown Object (File)
Tue, Dec 3, 2:25 AM
Unknown Object (File)
Wed, Nov 27, 10:04 AM
Unknown Object (File)
Wed, Nov 27, 10:04 AM
Unknown Object (File)
Wed, Nov 27, 10:04 AM
Unknown Object (File)
Wed, Nov 27, 10:03 AM

Details

Summary

Ref T4387. By using hg locate to attempt to only list files in the given path
browsing diffusion is a bit faster. In a repo of about 600M it shaves a rough 100ms
off viewing the root of the project.

Test Plan

Looked around in diffusion and saw it showed everything including .files, which was nice

Diff Detail

Repository
rP Phabricator
Branch
master
Lint
Lint Passed
Unit
No Test Coverage

Event Timeline

This is great! I think I caught one edge case inline -- does that make sense? Am I crazy? I didn't actually import my silly deeply-nested repository so I'm not 100% sure that this is actually an issue, but it seemed to have the wrong behavior when I poked it on the command line.

src/applications/diffusion/conduit/ConduitAPI_diffusion_browsequery_Method.php
204

I think we can't include "-X", because Mercurial won't print directories unless we enumerate files in them, so this will incorrectly hide directories which nest several levels deep without any files next to them. For example, I made a repository with just one file:

a/b/c/d/e/.../l/m

When browsing a/, we should see that b/ is a subdirectory. However, with the -X flag, it's not visible:

>>> orbital ~/repos/hg-deep $ hg status
A a/b/c/d/e/f/g/h/i/j/k/l/m
>>> orbital ~/repos/hg-deep $ hg locate -I ./a -X ./a/*/*/*
>>> orbital ~/repos/hg-deep $

Omitting the -X flag, we can see it:

>>> orbital ~/repos/hg-deep $ hg locate -I ./a
a/b/c/d/e/f/g/h/i/j/k/l/m

This is unfortunate because it means we have to do a lot of extra work at the root, but we could conceivably add caching to handle that case pretty easily.

Ah yes, I hadn't taken that case into account and you're absolutely right.

There is no efficient solution to doing this, but having the -I will at least make it faster for nested structures. I'll take another look at this next week to see if I can find a cleaner solution.

richardvanvelzen updated this revision to Unknown Object (????).Feb 7 2014, 5:39 PM

Remove the exclusion to include deeper nested structures

Closed by commit rP5771d1395203 (authored by Richard van Velzen <rvanvelzen@expert-shops.com>, committed by @epriestley).