Code review comment for lp:~jameinel/bzr/2.0b1-merge-sort

Revision history for this message
John A Meinel (jameinel) wrote :

This is a small tweak to how merge_sort counts 'merge_depth'. I think this is actually what we want. I believe Gary van der Merwe had noticed this in the past while working on qlog.

Basically the situation is (time going down):

 A
 |
 B-.
 |\ \
 | | C
 | |/
 | D
 |/
 F

The question is whether this should be logged as (time going up for log):

 F
   D
   C
 B
 A

or

 F
   D
     C
 B
 A

As C is a right-hand parent of D, I think it makes sense to increase its merge depth, even though the left-hand parent of D is a mainline ancestor.

Looking at the fix, I think it was just a matter of not paying attention to the:

                    if next_node_name in completed_node_names:
                        # this parent was completed by a child on the
                        # call stack. skip it.
                        continue

continue statement between when we pull off the left-hand parent and when we decide that the current node is a left-hand and thus doesn't increment the merge depth.

I'm working on an optimized Pyrex version, and so came across this.

« Back to merge proposal