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

[Matt Mackall]

On Wed, 2011-03-16 at 08:55 -0400, Greg Ward wrote:
> 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.

That's a bit better.

> (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


-- 
Mathematics is the supreme nostalgia of our time.