[MERGE] Sort knit fetches

[Vincent Ladeuil]

>>>>> "jam" == John Arbash Meinel <john at arbash-meinel.com> writes:

    jam> +def _get_total_build_size(keys, positions):

Just add a self parameter and you'll be able to assign it in the
classes that need it (and avoid the duplicated thunk for the two
classes).

Tests ?

<snip/>

    jam> @@ -1198,16 +1232,74 @@

<snip/>

    jam> +        Note that the keys are generally kept in the same order, so if you
    jam> +        present them in a sorted order, the sorting will generally be
    jam> +        preserved.

You're tempting the innocent there, either we guarantee an order
or we don't, if we don't, we can change our mind and the innocent
will suffer :)

<snip/>

    jam> +    def _group_keys_for_io(self, keys, non_local_keys, positions):

Tests ?


<snip/>

    jam> @@ -1334,13 +1426,11 @@
    jam>              # XXX: get_content_maps performs its own index queries; allow state
    jam>              # to be passed in.

I think you addressed '# XXX: Memory: TODO: batch data here to
cap buffered data at (say) 1MB', define a few lines above, delete it.

  
<snip/>
 
    jam> +    def _get_total_build_size(self, keys, positions):

See remark above.

The overall doesn't sounds complicated, but the manipulated data
is. I'm confident that any bug here may be caught by the readv
tests, but any bug will be harder to fix.

So that's a

BB:tweak

and decide for yourself if we need more tests. Localized ones
will be sufficient, I'm more worried about missed items (which
should be caught by readv tests) than wrongly grouped ones as the
later will reveal itself by its impact on performance.

        Vincent