From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-3.9 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by dcvr.yhbt.net (Postfix) with ESMTP id E73521F45E for ; Thu, 13 Feb 2020 18:40:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728076AbgBMSkf (ORCPT ); Thu, 13 Feb 2020 13:40:35 -0500 Received: from cloud.peff.net ([104.130.231.41]:42616 "HELO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1725781AbgBMSkf (ORCPT ); Thu, 13 Feb 2020 13:40:35 -0500 Received: (qmail 16541 invoked by uid 109); 13 Feb 2020 18:40:34 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with SMTP; Thu, 13 Feb 2020 18:40:34 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 15655 invoked by uid 111); 13 Feb 2020 18:49:30 -0000 Received: from coredump.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Thu, 13 Feb 2020 13:49:30 -0500 Authentication-Results: peff.net; auth=none Date: Thu, 13 Feb 2020 13:40:33 -0500 From: Jeff King To: Junio C Hamano Cc: git@vger.kernel.org Subject: Re: [PATCH 03/13] rev-list: fallback to non-bitmap traversal when filtering Message-ID: <20200213184033.GA15046@coredump.intra.peff.net> References: <20200213021506.GA1124607@coredump.intra.peff.net> <20200213021730.GC1126038@coredump.intra.peff.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On Thu, Feb 13, 2020 at 10:19:19AM -0800, Junio C Hamano wrote: > Jeff King writes: > > > The "--use-bitmap-index" option is usually aspirational: if we have > > bitmaps and the request can be fulfilled more quickly using them we'll > > do so, but otherwise fall back to a non-bitmap traversal. > > > > The exception is object filtering, which explicitly dies if the two > > options are combined. Let's convert this to the usual fallback behavior. > > > > This is a minor convenience for now (since the caller can easily know > > that --filter and --use-bitmap-index don't combine), but will become > > much more useful as we start to support _some_ filters with bitmaps, but > > not others. > > Makes sense. > > Perhaps the option should have been called allow-bitmap-index or > something along that line, but it is too late ;-) Yeah. It's also annoyingly long to type, and makes for long lines in the test scripts. ;) There are also some weird semantics with the fallback, because the output may differ depending on whether we use bitmaps (see one of the later patches). I wouldn't be opposed to cleaning this up and giving it a new option ("--allow-bitmaps" or something) to keep compatibility, but it's out of scope here. The existing option (and my suggestion, as well as most of the internal code) are guilty of equating "bitmap" with "object reachability bitmap". There's lots of things one might use bitmaps for, and at some point we might even expose such a thing via rev-list. Anyway, that concludes my rant. I don't think any of these are urgent to fix, and definitely shouldn't be part of this series. -Peff