All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Remi Galan Alfonso <remi.galan-alfonso@ensimag.grenoble-inp.fr>
Cc: William Duclot <william.duclot@ensimag.grenoble-inp.fr>,
	git@vger.kernel.org,
	simon rabourg <simon.rabourg@ensimag.grenoble-inp.fr>,
	francois beutin <francois.beutin@ensimag.grenoble-inp.fr>,
	antoine queru <antoine.queru@ensimag.grenoble-inp.fr>,
	matthieu moy <matthieu.moy@grenoble-inp.fr>,
	mhagger@alum.mit.edu
Subject: Re: [PATCH 0/2] strbuf: improve API
Date: Wed, 1 Jun 2016 03:42:18 -0400	[thread overview]
Message-ID: <20160601074218.GB14096@sigill.intra.peff.net> (raw)
In-Reply-To: <1069084553.156626.1464607928755.JavaMail.zimbra@ensimag.grenoble-inp.fr>

On Mon, May 30, 2016 at 01:32:08PM +0200, Remi Galan Alfonso wrote:

> William Duclot <william.duclot@ensimag.grenoble-inp.fr> writes:
> > This patch series implements an improvment of the strbuf API, allowing
> > strbuf to use preallocated memory. This makes strbuf fit to be used
> > in performance-critical operations.
> > 
> > The first patch is simply a preparatory work, adding tests for
> > existing strbuf implementation.
> > Most of the work is made in the second patch: handle pre-allocated
> > memory, extend the API, document it and test it.
> 
> Seems interesting, however do you have any test/example that would
> show the difference of performance between using these optimizations
> and not using them?
> 
> Such values would make a nice addition to help convince people that
> your series is interesting to have and use.

I'll second the request for actual numbers. I'm a little dubious that
malloc overhead is actually a significant place we are spending time, or
if there is simply a superstitious avoidance of using strbufs. A huge
number of strbufs are used for filenames, where we're about to make a
syscall anyway. If your allocator for a 4K page is not competitive with
a context switch, I suspect the best solution is to get a new allocator.

So I wonder if we have some less-invasive alternatives:

  1. Ship a faster allocator library with git, and use its malloc by
     default.

  2. Do caching tricks for strbufs used in tight loops. For example,
     have strbuf_release() throw its buffer into a last-used cache, and
     let the next strbuf_grow() use that cache entry. This cuts malloc()
     out of the loop.

     You'd probably want to protect the cache with a mutex, though. Most
     of git isn't thread-safe, but a few parts are, and strbufs are
     low-level enough that they might get called.

I have no idea if those ideas would work. But I wouldn't want to start
looking into either of them without some idea of how much time we're
actually spending on strbuf mallocs (or how much time we would spend if
strbufs were used in some proposed sites).

-Peff

  reply	other threads:[~2016-06-01  7:42 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-30 10:36 [PATCH 0/2] strbuf: improve API William Duclot
2016-05-30 10:36 ` [PATCH 1/2] strbuf: add tests William Duclot
2016-05-30 11:26   ` Johannes Schindelin
2016-05-30 13:42     ` Simon Rabourg
2016-05-30 11:56   ` Matthieu Moy
2016-05-31  2:04   ` Michael Haggerty
2016-05-31  9:48     ` Simon Rabourg
2016-05-30 10:36 ` [PATCH 2/2] strbuf: allow to use preallocated memory William Duclot
2016-05-30 12:13   ` Johannes Schindelin
2016-05-30 13:20     ` William Duclot
2016-05-31  6:21       ` Johannes Schindelin
2016-05-31  3:05     ` Michael Haggerty
2016-05-31  6:41       ` Johannes Schindelin
2016-05-31  8:25         ` Michael Haggerty
2016-05-30 12:52   ` Matthieu Moy
2016-05-30 14:15     ` William Duclot
2016-05-30 14:34       ` Matthieu Moy
2016-05-30 15:16         ` William Duclot
2016-05-31  4:05     ` Michael Haggerty
2016-05-31 15:59       ` William Duclot
2016-06-03 14:04       ` William Duclot
2016-05-30 21:56   ` Mike Hommey
2016-05-30 22:46     ` William Duclot
2016-05-30 22:50       ` Mike Hommey
2016-05-31  6:34   ` Junio C Hamano
2016-05-31 15:45     ` William
2016-05-31 15:54       ` Matthieu Moy
2016-05-31 16:08         ` William Duclot
2016-05-30 11:32 ` [PATCH 0/2] strbuf: improve API Remi Galan Alfonso
2016-06-01  7:42   ` Jeff King [this message]
2016-06-01 19:50     ` David Turner
2016-06-01 20:09       ` Jeff King
2016-06-01 20:22         ` David Turner
2016-06-01 21:07     ` Jeff King
2016-06-02 11:11       ` Michael Haggerty
2016-06-02 12:58         ` Matthieu Moy
2016-06-02 14:22           ` William Duclot
2016-06-24 17:20         ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160601074218.GB14096@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=antoine.queru@ensimag.grenoble-inp.fr \
    --cc=francois.beutin@ensimag.grenoble-inp.fr \
    --cc=git@vger.kernel.org \
    --cc=matthieu.moy@grenoble-inp.fr \
    --cc=mhagger@alum.mit.edu \
    --cc=remi.galan-alfonso@ensimag.grenoble-inp.fr \
    --cc=simon.rabourg@ensimag.grenoble-inp.fr \
    --cc=william.duclot@ensimag.grenoble-inp.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.