[PATCH 1 of 3] dirstate: factor maybelookup() out of write()

Greg Ward greg at gerg.ca
Wed Mar 16 07:55:33 CDT 2011


On 15 March 2011, Matt Mackall said:
> On Tue, 2011-03-15 at 21:39 -0400, Greg Ward wrote:
> > # HG changeset patch
> > # User Greg Ward <greg at gerg.ca>
> > # Date 1300239394 14400
> > # Node ID c5fa123f60a63af19057e873106c532ee2c553fa
> > # Parent  0652b2da832daada866a68f7a4359227570c2447
> > dirstate: factor maybelookup() out of write().
> > 
> > For the moment, this serves no obvious purpose.  But it will be needed
> > in localrepository.commit() to avoid a subtle dirstate race when there
> > are multiple commits in the same second from the same process.
> 
> FYI, this is rather performance-sensitive due to being in the inner loop
> of the dirstate write function. You'll note that that function goes to
> some pains to avoid various lookups. It might be better to have it run
> on the whole set of files to amortize the call overhead..

Argh.  My initial implementation did just that, but I talked myself
into taking only a single filename, leaving the loop in the caller,
because that's how every other state-changing method in dirstate
looks.  And it means write() would effectively loop twice:

  self.maybelookup(self._map, now)     # loop over self._map
  for (f, e) in self._map.iteritems(): # loop again
      if f in copymap:
          ...

Maybe I'll add something to contrib/perf.py to measure
dirstate.write() and see the effects of this change.

BTW: what's your feeling on the name 'maybelookup()'?  Adrian has
*juuuust* about succeeded in convincing me to go with checkmtime(). Or
checkmtimes() if it takes an iterable of filenames.

(My rationale for 'maybelookup()' is not that that this method might
do the "lookup", i.e. compare file states, but that it might change
f's state so that future code will need to do the lookup.  I know some
Java people who would call this maybePutFileInStateThatRequires-
ComparingContentLater(), but they're obviously insane.)

        Greg


More information about the Mercurial-devel mailing list