[PATCH 4 of 7 V6] dirstate: attach the nonnormalset to a propertycache

[Martin von Zweigbergk]

On Tue, Dec 22, 2015 at 11:09 AM Laurent Charignon <lcharignon at fb.com>
wrote:

> On Dec 22, 2015, at 8:23 AM, Martin von Zweigbergk <martinvonz at google.com>
> wrote:
>
> On Mon, Dec 21, 2015 at 4:36 PM Laurent Charignon <lcharignon at fb.com>
> wrote:
>
>> # HG changeset patch
>> # User Laurent Charignon <lcharignon at fb.com>
>> # Date 1450743882 28800
>> #      Mon Dec 21 16:24:42 2015 -0800
>> # Node ID d3209dc5c211a7c937b68a4712dd77bc3ec978e1
>> # Parent  1be21e4798cd66383e8f63eafe84a3e32ef4b8d9
>> dirstate: attach the nonnormalset to a propertycache
>>
>> This patch attaches the nonnormalset to a property cache so that we build
>> it
>> only when needed.
>>
>> diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
>> --- a/mercurial/dirstate.py
>> +++ b/mercurial/dirstate.py
>> @@ -124,6 +124,11 @@ class dirstate(object):
>>          return self._copymap
>>
>>      @propertycache
>> +    def _nonnormalset(self):
>> +        self._read()
>> +        return self._nonnormalset
>>
>
> I would have expected "return self.nonnomalentries(self._dmap)".
>
>
> Calling self._read() assigns self._nonnormalset from the dmap already
>

Yes, it will work, but it's a little different from how it is usually done.
I now see that the two @properycache fields, _map and _copymap, are also
done that way here. The reason they look that way is because they are
created by the same function (parse_dirstate). There seems to be no reason
that _nonnormalset needs to assigned at the same time, so you should be
able to do what I suggested above here (besides the typos), which would
mean that _nonnormalset would not be calculated unless it was accessed. It
would still depend on _map, since it passes that into nonnormalentries, so
_map's propertycache function will take care of that. I hope that makes
sense.


>
> @propertycache makes the first access to dirstate._nonnormalset call this
> function, then it replaces the function by the returned value, so future
> calls will just access the value. You can (and should) probably also remove
> the assignment of self._nonnormalset in the initializer.
>
>
> What are you referring to?
> self._nonnormalset is not assigned in the initializer, it is assigned in
> two places _writedirstate, clear and _read the same places that self._map
> is assigned.
>

My point was that I think the dirstate object will have a _nonnormalset
field that replaces (or hides (?); I don't know Python well) the
_nonnormalset defined on the class (the one annotated with @propertycache).
So I wonder if the property cache function is ever called.

A more practical way of phrasing my question is: What fails if you insert
"raise 'hell'" at the beginning of the method?


>
> I don't know Python well enough to say for sure, but I'd guess that
> replaces the self._nonnormalset object created by @propertycache.
>
>
>
>> +
>> +    @propertycache
>>      def _filefoldmap(self):
>>          try:
>>              makefilefoldmap = parsers.make_file_foldmap
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel at selenic.com
>> https://selenic.com/mailman/listinfo/mercurial-devel
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20151222/459690ee/attachment.html>