[PATCH 5 of 8] obscache: add a cache for 1/2 of the "obsolete" property

Augie Fackler raf at durin42.com
Fri May 19 18:32:53 EDT 2017


> On May 19, 2017, at 15:15, Pierre-Yves David <pierre-yves.david at ens-lyon.org> wrote:
> 
> 
> 
> On 05/19/2017 11:53 PM, Augie Fackler wrote:
>> On Fri, May 19, 2017 at 04:28:04PM +0200, Pierre-Yves David wrote:
>>> # HG changeset patch
>>> # User Pierre-Yves David <pierre-yves.david at octobus.net>
>>> # Date 1495197986 -7200
>>> #      Fri May 19 14:46:26 2017 +0200
>>> # Node ID c2b1c81b3c4cba8cc21b1a1b92f4e34e6aa1807a
>>> # Parent  b5c984cd0640255b94f22d56a4bfe398b2c5de2e
>>> # EXP-Topic obscache
>>> # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>>> #              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r c2b1c81b3c4c
>>> obscache: add a cache for 1/2 of the "obsolete" property
>> 
>> So, can you point me to other (upcoming) concrete subclasses of
>> dualsourcecache, or is this the only one?
> 
> There are current a second user of this abstract class in evolve. It is used for obsdiscovery[1]. And I expect to we'll experiment with more of this class of cache in the near future.
> 
> [1] https://www.mercurial-scm.org/repo/evolve/file/71d242e0fc1a/hgext3rd/evolve/obsdiscovery.py#l440
> 
> The logic in the cache is easy to screw up and the end result is really need to use (implement save/load/reset/update(revs, obs)). Having a class ready to use is -really- handy.

I wish it could be done without the use of inheritance, but it's fine-ish for now as an abstract class. Should it perhaps be using the abc module to be a more explicit base class?

(I haven't ever used abc, so "no" or "not now" is fine)

> In addition, there are final stage of skip-obsstore optimization that requires some rework and collaboration in core something hard to access from an extension.
> 
> So, having this intermediate abstract class is a corner-stone of the current contribution. (most of work in evolve 6.2 is actually about extracting and reusing this logic)
> 
>> Also, could we put these big new classes in obscache.py or some other
>> new file? I'm wary of adding more code to the already-1200-lines
>> obsolete.py, and since these are coming from evolve I'm hoping that
>> they're still easily separable from obsolete.py...
> 
> This crossed my mind when adding it. However, there are significant improvement to that class arriving soon. These improvement will need to access (and refactor) some lower logic around reading markers and al. I think it is wiser to wait for the dust to settle after these upgrade and cut where this is

I'm not really satisfied with this answer. Can you split things up into a better organization scheme so we don't end up with more of a mess in this already far-too-large module? That'd make things easier for me to reason about.

> (the version in evolve access some "_private" method and duplicates some code)

^ this is a sign to me that some more refactoring should probably happen before this lands, as much as I want it now.

> 
> Cheers,
> 
> -- 
> Pierre-Yves David



More information about the Mercurial-devel mailing list