Patchwork [1,of,4,stable] largefiles: introduce basic debugstate --large functionality

login
register
mail settings
Submitter Mads Kiilerich
Date Dec. 21, 2012, 6:56 p.m.
Message ID <7ccb2671617299c81b9e.1356116161@mk-desktop>
Download mbox | patch
Permalink /patch/245/
State Accepted
Commit e16982a74bf71404abf4c0e1b7899dd753df69ee
Headers show

Comments

Mads Kiilerich - Dec. 21, 2012, 6:56 p.m.
# HG changeset patch
# User Mads Kiilerich <madski at unity3d.com>
# Date 1356115949 -3600
# Branch stable
# Node ID 7ccb2671617299c81b9ed9f38e3debc7231ebfc4
# Parent  777084ac84167e3bdea45b5c00de1106cca36eef
largefiles: introduce basic debugstate --large functionality

Very useful for debugging largefiles "performance" issues.

The added test exposes an issue that will be solved next.
Kevin Bullock - Dec. 21, 2012, 7:10 p.m.
On Dec 21, 2012, at 12:56 PM, Mads Kiilerich wrote:

> # HG changeset patch
> # User Mads Kiilerich <madski at unity3d.com>
> # Date 1356115949 -3600
> # Branch stable
> # Node ID 7ccb2671617299c81b9ed9f38e3debc7231ebfc4
> # Parent  777084ac84167e3bdea45b5c00de1106cca36eef
> largefiles: introduce basic debugstate --large functionality
> 
> Very useful for debugging largefiles "performance" issues.

+1 on the general direction.

> The added test exposes an issue that will be solved next.

So the test result in the patch is incorrect? Or it's correct but too slow?

> diff --git a/tests/test-largefiles.t b/tests/test-largefiles.t
> --- a/tests/test-largefiles.t
> +++ b/tests/test-largefiles.t
> @@ -17,8 +17,8 @@
>> EOF
> 
> Create the repo with a couple of revisions of both large and normal
> -files, testing that status correctly shows largefiles and that summary output
> -is correct.
> +files, testing that status correctly shows largefiles and and updates
> +dirstate and that summary output is correct.

Big long run-on sentence, and a doubled 'and' ---------- ^^^^^^^

pacem in terris / ??? / ?????? / ????????? / ??
Kevin R. Bullock
Mads Kiilerich - Dec. 21, 2012, 8:26 p.m.
On 12/21/2012 08:10 PM, Kevin Bullock wrote:
> On Dec 21, 2012, at 12:56 PM, Mads Kiilerich wrote:
>
>> # HG changeset patch
>> # User Mads Kiilerich <madski at unity3d.com>
>> # Date 1356115949 -3600
>> # Branch stable
>> # Node ID 7ccb2671617299c81b9ed9f38e3debc7231ebfc4
>> # Parent  777084ac84167e3bdea45b5c00de1106cca36eef
>> largefiles: introduce basic debugstate --large functionality
>>
>> Very useful for debugging largefiles "performance" issues.
> +1 on the general direction.
>
>> The added test exposes an issue that will be solved next.
> So the test result in the patch is incorrect? Or it's correct but too slow?

The test result is correct in the way that it shows the current 
kind-of-working behaviour and the test passes.

The test result also points out why status currently often is slow.

- but I guess I just should leave this one out from stable and push the 
actual status fix on stable without a test and push this test to default.

/Mads
Kevin Bullock - Dec. 21, 2012, 8:50 p.m.
On Dec 21, 2012, at 2:26 PM, Mads Kiilerich wrote:

> On 12/21/2012 08:10 PM, Kevin Bullock wrote:
>> On Dec 21, 2012, at 12:56 PM, Mads Kiilerich wrote:
>> 
>>> # HG changeset patch
>>> # User Mads Kiilerich <madski at unity3d.com>
>>> # Date 1356115949 -3600
>>> # Branch stable
>>> # Node ID 7ccb2671617299c81b9ed9f38e3debc7231ebfc4
>>> # Parent  777084ac84167e3bdea45b5c00de1106cca36eef
>>> largefiles: introduce basic debugstate --large functionality
>>> 
>>> Very useful for debugging largefiles "performance" issues.
>> +1 on the general direction.
>> 
>>> The added test exposes an issue that will be solved next.
>> So the test result in the patch is incorrect? Or it's correct but too slow?
> 
> The test result is correct in the way that it shows the current kind-of-working behaviour and the test passes.
> 
> The test result also points out why status currently often is slow.
> 
> - but I guess I just should leave this one out from stable and push the actual status fix on stable without a test and push this test to default.

I'm not sure that's the approach I'd take. It'd be nice if there were _some_ way to expose the issue before the fix lands, and bundle the test to ensure it's fixed into the fix itself.

Maybe you could drive the test using inline Python on stable, and then change it to use 'debugstate --large' on default?

pacem in terris / ??? / ?????? / ????????? / ??
Kevin R. Bullock
Mads Kiilerich - Dec. 28, 2012, 11:04 a.m.
Series pushed to crew. Thanks for reviewing.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20121228/1a5ae720/attachment.html>

Patch

diff --git a/hgext/largefiles/lfcommands.py b/hgext/largefiles/lfcommands.py
--- a/hgext/largefiles/lfcommands.py
+++ b/hgext/largefiles/lfcommands.py
@@ -383,6 +383,13 @@ 
     store = basestore._openstore(repo)
     return store.verify(revs, contents=contents)
 
+def debugdirstate(ui, repo):
+    '''Show basic information for the largefiles dirstate'''
+    lfdirstate = lfutil.openlfdirstate(ui, repo)
+    for file_, ent in sorted(lfdirstate._map.iteritems()):
+        mode = '%3o' % (ent[1] & 0777 & ~util.umask)
+        ui.write("%c %s %10d %s\n" % (ent[0], mode, ent[2], file_))
+
 def cachelfiles(ui, repo, node, filelist=None):
     '''cachelfiles ensures that all largefiles needed by the specified revision
     are present in the repository's largefile cache.
diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -254,6 +254,13 @@ 
         result = result or lfcommands.verifylfiles(ui, repo, all, contents)
     return result
 
+def overridedebugstate(orig, ui, repo, *pats, **opts):
+    large = opts.pop('large', False)
+    if large:
+        lfcommands.debugdirstate(ui, repo)
+    else:
+        orig(ui, repo, *pats, **opts)
+
 # Override needs to refresh standins so that update's normal merge
 # will go through properly. Then the other update hook (overriding repo.update)
 # will get the new files. Filemerge is also overridden so that the merge
diff --git a/hgext/largefiles/uisetup.py b/hgext/largefiles/uisetup.py
--- a/hgext/largefiles/uisetup.py
+++ b/hgext/largefiles/uisetup.py
@@ -59,6 +59,11 @@ 
                      _('verify largefile contents not just existence'))]
     entry[1].extend(verifyopt)
 
+    entry = extensions.wrapcommand(commands.table, 'debugstate',
+                                   overrides.overridedebugstate)
+    debugstateopt = [('', 'large', None, _('display largefiles dirstate'))]
+    entry[1].extend(debugstateopt)
+
     entry = extensions.wrapcommand(commands.table, 'outgoing',
         overrides.overrideoutgoing)
     outgoingopt = [('', 'large', None, _('display outgoing largefiles'))]
diff --git a/tests/test-largefiles.t b/tests/test-largefiles.t
--- a/tests/test-largefiles.t
+++ b/tests/test-largefiles.t
@@ -17,8 +17,8 @@ 
   > EOF
 
 Create the repo with a couple of revisions of both large and normal
-files, testing that status correctly shows largefiles and that summary output
-is correct.
+files, testing that status correctly shows largefiles and and updates
+dirstate and that summary output is correct.
 
   $ hg init a
   $ cd a
@@ -35,6 +35,17 @@ 
   A normal1
   A sub/large2
   A sub/normal2
+  $ touch large1 sub/large2
+  $ sleep 1
+  $ hg st
+  $ hg debugstate --nodates
+  n 644         41 .hglf/large1
+  n 644         41 .hglf/sub/large2
+  n 644          8 normal1
+  n 644          8 sub/normal2
+  $ hg debugstate --large
+  n   0         -1 large1
+  n   0         -1 sub/large2
   $ echo normal11 > normal1
   $ echo normal22 > sub/normal2
   $ echo large11 > large1