[PATCH 7 of 9] store: replace invocation of "getsize()" by "vfs.stat()"

[FUJIWARA Katsunori]

At Mon, 08 Oct 2012 22:57:24 +0200,
Adrian Buehlmann wrote:
> 
> On 2012-10-08 19:06, FUJIWARA Katsunori wrote:
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
> > # Date 1349714515 -32400
> > # Node ID 6353632376a81896afadcef810da1cf1c3c6d56f
> > # Parent  ef7a5af78db825a4e8f00f7f6a65d0b509a3eeb5
> > store: replace invocation of "getsize()" by "vfs.stat()"
> > 
> > This patch replaces invocation of "getsize()", which calls "os.stat()"
> > internally, by "vfs.stat()".
> > 
> > The object referred by "self.rawvfs" is used internally by
> > "_fncachevfs" and doesn't encode filename for each file API invocation.
> > 
> > This patch invokes "os.stat()" via "self.rawvfs" to avoid redundant
> > filename encoding: invocation of "os.stat()" via "self.vfs" hides
> > filename encoding and encoding result from caller, so it is not
> > appropriate, when both encoded and non-encoded filenames should be
> > yield.
> > 
> > Even though changeset b42b0729744d improved stream_out performance by
> > "self.pathsep + path", this patch replaces it by
> > "os.path.join(self.base, path)" of vfs. So, this may increase cost to
> > join path components.
> > 
> > But this shouldn't have large impact, because:
> > 
> >   - such cost is much less than cost of "os.stat()" which causes
> >     system call invocation
> > 
> >   - "datafiles()" of store object is invoked only for "hg manifest
> >     --all" or "hg verify" which are both heavy functions
> > 
> > diff -r ef7a5af78db8 -r 6353632376a8 mercurial/store.py
> > --- a/mercurial/store.py	Tue Oct 09 01:41:55 2012 +0900
> > +++ b/mercurial/store.py	Tue Oct 09 01:41:55 2012 +0900
> > @@ -459,6 +459,7 @@
> >          self.pathsep = self.path + '/'
> >          self.createmode = _calcmode(vfs)
> >          vfs.createmode = self.createmode
> > +        self.rawvfs = vfs
> >          fnc = fncache(vfs)
> >          self.fncache = fnc
> >          self.vfs = _fncachevfs(vfs, fnc, encode)
> > @@ -467,16 +468,14 @@
> >      def join(self, f):
> >          return self.pathsep + self.encode(f)
> >  
> > -    def getsize(self, path):
> > -        return os.stat(self.pathsep + path).st_size
> > -
> 
> This backouts d592759aabc7 by Bryan ("store: abstract out how we
> retrieve a file's size")
> 
> I think he did this for his packrepo extension:
> http://markmail.org/message/fip3ca2xre7g3tf2

Oops, sorry for backing it out, Bryan.

I couldn't notice it, because there is no code to use it yet. I had to
check not only d592759aabc7 itself but also revisions around it.

This patch was already queued to default branch. Should I post the
patch to redo abstracting getting file size out ?

> >      def datafiles(self):
> >          rewrite = False
> >          existing = []
> > +        getstat = self.rawvfs.stat
> >          for f in sorted(self.fncache):
> >              ef = self.encode(f)
> >              try:
> > -                yield f, ef, self.getsize(ef)
> > +                yield f, ef, getstat(ef).st_size
> >                  existing.append(f)
> >              except OSError, err:
> >                  if err.errno != errno.ENOENT:
> 
> 


----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy at lares.dti.ne.jp