git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michael Haggerty <mhagger@alum.mit.edu>
To: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Cc: git <git@vger.kernel.org>
Subject: Re: Project idea: strbuf allocation modes
Date: Tue, 22 Apr 2014 11:09:20 +0200	[thread overview]
Message-ID: <535631C0.2020100@alum.mit.edu> (raw)
In-Reply-To: <vpqr44psuzf.fsf@anie.imag.fr>

On 04/22/2014 09:07 AM, Matthieu Moy wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>>     STRBUF_OWNS_MEMORY
>>
>>         The memory pointed to by buf is owned by this strbuf.  If this
>>         bit is not set, then the memory should never be freed, and
>>         (among other things) strbuf_detach() must always call xstrcpy().
> 
> I just foresee a small difficulty from the caller side:
> 
>             char path_buf[PATH_MAX];
>             char *detached;
> 
>             struct strbuf path;
> 
>             strbuf_wrap_preallocated(&path, path_buf, 0, sizeof(path));
>             ...
>             detached = strbuf_detach(&path);
> 
> the wrapping/unwrapping of path_buf magically turned a stack-allocated
> buffer into a heap-allocated one. And the initial goal of avoiding
> malloc() is defeated. So, essentially, one should avoir using
> strbuf_wrap_preallocated with strbuf_detach, right?

Correct; it wouldn't bring any performance advantages.  But
strbuf_detach() should be implemented for such strbufs for completeness
so that after the strbuf is instantiated, users don't have to know its
allocation mode.

>                                                      But I agree with
> Junio that if the API is properly used, everything should work. I'm just
> worried that we will add a bit more complexity to the API, and I'm not
> sure we can actually expect noticeable improvements in terms of
> performance (i.e. do we actually use strbuf for performance-critical
> stuff?).

The whole point of the change is to *allow* strbuf to be used in
performance-critical stuff.

> In your proposal, would STRBUF_OWNS_MEMORY be a constant, or a flag that
> change when the internal buffer needs reallocation? My understanding is
> that it should change (if STRBUF_FIXED_MEMORY is not set), and the
> strbuf wrapping a preallocated buffer would become a "normal" strbuf
> when its internal buffer grows.

Correct.  STRBUF_OWNS_MEMORY itself is of course a constant like 0x02 (a
mask to set/clear a bit in the flags) but the corresponding flags bit
would sometimes be changed internal to the strbuf implementation.  For
example, if the buffer is grown past its original size (when
STRBUF_FIXED_MEMORY is not set) the bit would be set.

After strbuf_detach(), there would be no automatic way to re-attach such
a strbuf to its original heap-allocated memory.  So in this situation
the buffer would have to be pointed at strbuf_slopbuf again, and the
STRBUF_OWNS_MEMORY bit would be cleared again.

> If so, your "strbuf_detach() must always call xstrcpy()" is to be
> understood as "if STRBUF_OWNS_MEMORY is still set when strbuf_detach()
> is called, then it must always call xstrcpy()", right?

Exactly.


How does the size of this project compare to what you are looking for
for your Ensimag students?

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

  reply	other threads:[~2014-04-22  9:09 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-08 15:44 Students project on Git (Ensimag) Matthieu Moy
2014-04-18 13:50 ` Project idea: strbuf allocation modes Michael Haggerty
2014-04-18 17:21   ` Junio C Hamano
2014-04-18 20:04     ` Michael Haggerty
2014-04-19  0:55       ` Junio C Hamano
2014-04-22  7:07   ` Matthieu Moy
2014-04-22  9:09     ` Michael Haggerty [this message]
2014-04-22 10:38       ` Matthieu Moy

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=535631C0.2020100@alum.mit.edu \
    --to=mhagger@alum.mit.edu \
    --cc=Matthieu.Moy@grenoble-inp.fr \
    --cc=git@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).