Re: [PATCH v2 4/9] list-objects-filter: implement composite filters

From: Matthew DeVore <matvore@comcast.net>
Date: Jun 6, 2019, 22:32 UTC
Message-ID: <20190606223251.GA7246@comcast.net>
In-Reply-To: <0005347e-ceed-ac9e-ad0d-b7b11bc55d38@jeffhostetler.com>

On Mon, Jun 03, 2019 at 05:51:28PM -0400, Jeff Hostetler wrote:

Show 6 quoted lines

> Since we are assuming 'compose' is an AND operation, there may be an
> opportunity to short-cut some of this loop for blobs.  That is, if the
> object is a blob and any filter rejects it, it is omitted, so we don't
> need to keep looping for that object.  (Tree objects cannot be short-cut
> this way because a tree may appear at different depths or in different
> sparse "cones" and may have to be reconsidered.)

Blobs are also treated almost the same way as tree objects in tree:<depth> filters - they can be included by tree:<depth> - so they also need to be reconsidered when found at different depths.

But I agree it's always true that if some prior filter has excluded a blob, the later filters don't even need to be *called at all* for that blob, unless perhaps it's found under a different tree later. I also think it may be too early to implement this optimization, since filter in a later release may just want to "know" about a blob even if it must be excluded in the final result.

Does the optimization apply to trees as well? Does a tree:<depth> filter still want to consider children of tree X if tree X has already been excluded by another filter? If it doesn't want to consider, we can short-circuit the checks very aggressively. If it does want to consider, we want the short-circuiting to be customizable at least for trees.

A minor point - I don't think that short-circuiting the for loop (breaking out early) is important, since it will be very rare that a combine: filter has more than 4 or so sub-filters anyway. Calling the filter_fn implementation and letting that do internal short-circuiting (informed by the previous filters' results) can, however, skip a lot of computation.

> So you could add an "affects blobs only" bit to the per-filter data
> and try this out.  For example a "compose:blob:none+sparse:foo" should
> perform better than "compose:sparse:foo+blob:none" but give the same
> results.

Does "affects blobs only" mean the filter includes all non-blob objects?