nested trees design-approach : composite trees vs iter_changes

Ian Clatworthy ian.clatworthy at internode.on.net
Wed May 6 09:48:08 BST 2009


Robert Collins wrote:
> On Tue, 2009-05-05 at 10:19 -0400, Aaron Bentley wrote:

>> Instead, you have been undermining me.  I don't like being vetoed, but
>> it is better than being undermined.  It is at least direct.  It
>> demonstrates that your objections are strong enough that you're willing
>> to derail the current development over them, and that's important data.
> 
> Concerns that aren't addressed rarely just go away; trying to get it
> discussed wasn't intended to be undermining, but an attempt to get it
> discussed!

And I certainly took your email in that light. From my perspective, the
primary issue was the relatively late veto/articulation of your
concerns. It's *extremely* frustrating to do a pile of work, go through
multiple reviews and then have something vetoed a month or two afterwards.

As a project, we need to recognise this as a process issue and work out
a better way of working together. It's happened before to me with the
branch-specific rules code. That got as far as landing into 1.6 and was
backed out with promises of providing better infrastructure to base a
solution on. 9 months on, we *still* have no solution, no better
infrastructure and the proposed replacement design has hit a wall. In
case it isn't obvious, it ticks me off on multiple levels. To begin
with, we replaced a working solution with no solution (and I'll be
furious if we end up doing the same thing here).

>> It is an attempt to get coverage of a bunch of commands with minimal
>> cost.  It is a stepping stone, not a long-term solution.
> 
> What is the long term solution? http://bazaar-vcs.org/NestedTreesDesign
> doesn't talk about other options.

I'd like to see Aaron update that page real soon now.

>>>  * I don't think it hides whats going on any more or less than having a 
>>>    separate class which pulls data out of all the children.
>> I usually find your code to be very, very unclear, so I do not place
>> much stock in your opinion here.
> 
> Bringing a technical discussion down to ad hominems is rarely helpful.
> At best it leaves people feeling defensive or offended.

Yes, I don't think the comment was warranted or helpful. All of us are
guilty of bad code now and then but everyone in the core team writes
pretty good code most of the time. Even me I hope. :-)

FWIW, I'm pretty strict in my reviews about code quality and clarity and
the various approved patches are certainly up to scratch. I also think
there's value in a layered approach with respect to clarity and
debugging, i.e.:

1. having a simple Tree class as a building block
2. having a higher level Tree class that adds the complexity and
   smart caches needed for nested trees.

Right now, these are called Tree and CompositeTree. In the future, Tree
might gain the smarts in CompositeTree but then I'd like us to consider
renaming the current Tree class to something like PlainTree rather than
making Tree a mega-complex class (ala Repository).

BTW, the division of responsibility makes it very easy to ensure you
only pay for nested trees when they exist. Aaron has an API called
maybe_composite() that hands you back a plain tree or composite tree
based on what's needed. This is only used for 'read' operations like
diff and status. Stuff like merge and revert don't want the facade and
walk iter_references() as required.

I'm not sure whether maybe_composite() should be a separate API to
open_workingtree or not but I do find the class layering useful in terms
of design clarity. Personally, I'm not convinced simply adding recursion
is enough for good performance. We had that 2 months back and had to
stop --development-rich-root from supporting subtrees because it was a
performance regression that couldn't easily be fixed.

So, trying to keep this feature moving forward consistent with poolie's
suggestions (see his mail), I'd like to see:

1. Aaron update the NestedTreeDesign wiki page. I'll then turn it into a
   doc/developers document and we can debate the overall design further.

2. Some draft userdoc. I'll draft it if no-one has done that already.
   That will undoubtedly uncover some more things and challenges for
   us to consider, e.g. which commands need a recurse-vs-otherwise
   option.

3. A decision on whether having unique file-ids across trees is an
   acceptable initial limitation. If so, we need a plan as to when and
   how we check for non-uniqueness. If not, I can't see nested trees
   making it by 2.0, so it's 3.0 (12 months say) before we'll have
   something for end users (given our desire for just one format during
   2.0).

4. poolie propose some process improvements so things don't
   reach the frustration levels we're seeing here again.

Ian C.



More information about the bazaar mailing list