From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EE4ADC56202 for ; Tue, 24 Nov 2020 02:48:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 9E63920719 for ; Tue, 24 Nov 2020 02:48:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728776AbgKXCsX (ORCPT ); Mon, 23 Nov 2020 21:48:23 -0500 Received: from cloud.peff.net ([104.130.231.41]:39678 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726508AbgKXCsX (ORCPT ); Mon, 23 Nov 2020 21:48:23 -0500 Received: (qmail 30872 invoked by uid 109); 24 Nov 2020 02:48:23 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 24 Nov 2020 02:48:23 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 10624 invoked by uid 111); 24 Nov 2020 02:48:22 -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; Mon, 23 Nov 2020 21:48:22 -0500 Authentication-Results: peff.net; auth=none Date: Mon, 23 Nov 2020 21:48:22 -0500 From: Jeff King To: Taylor Blau Cc: Junio C Hamano , git@vger.kernel.org, dstolee@microsoft.com Subject: Re: [PATCH 01/23] ewah/ewah_bitmap.c: grow buffer past 1 Message-ID: References: <36deaad366d66d10b96755dd6969bfe51123a2d4.1605123652.git.me@ttaylorr.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On Mon, Nov 23, 2020 at 11:22:04AM -0500, Taylor Blau wrote: > > I think this probably mimics what alloc_nr() computes for ALLOC_GROW(). > > I wonder why buffer_grow() cannot be built around ALLOC_GROW() instead? > > I think that we probably could just use ALLOC_GROW() as you suggest. > Funny enough, reading through GitHub's chat logs, apparently this is > something that Peff and I talked about. So, 16 probably came from > alloc_nr(), but we probably stopped short of realizing that we could > just use ALLOC_GROW as-is. That would probably be OK. It's a bit more aggressive, which could matter if you have a large number of very small bitmaps. My original goal of the "grow less aggressively" patch was to keep memory usage down, knowing that I was going to be holding a lot of bitmaps in memory at once. But even with micro-optimizations like this, it turned out to be far too big in practice (and hence Stolee's work on top to reduce the total number we hold at once). > @@ -33,12 +34,7 @@ static inline size_t max_size(size_t a, size_t b) > static inline void buffer_grow(struct ewah_bitmap *self, size_t new_size) > { > size_t rlw_offset = (uint8_t *)self->rlw - (uint8_t *)self->buffer; > - > - if (self->alloc_size >= new_size) > - return; > - > - self->alloc_size = new_size; > - REALLOC_ARRAY(self->buffer, self->alloc_size); > + ALLOC_GROW(self->buffer, new_size, self->alloc_size); > self->rlw = self->buffer + (rlw_offset / sizeof(eword_t)); > } I think the real test would be measuring the peak heap of the series as you posted it in v2, and this version replacing this patch (and the "grow less aggressively" one) with ALLOC_GROW(). On something big, like repacking all of the torvalds/linux or git/git fork networks. If there's no appreciable difference, then definitely I think it's worth the simplicity of reusing ALLOC_GROW(). -Peff