Hyrum K Wright wrote on Fri, Jan 20, 2012 at 06:59:45 -0600:
> On Fri, Jan 20, 2012 at 12:42 AM, Greg Stein <gstein_at_gmail.com> wrote:
> >
> > On Jan 19, 2012 11:02 PM, "Daniel Shahaf" <danielsh_at_elego.de> wrote:
> >>
> >> Hyrum K Wright wrote on Thu, Jan 19, 2012 at 21:47:51 -0600:
> >> > On Thu, Jan 19, 2012 at 8:40 PM, Daniel Shahaf <danielsh_at_elego.de>
> >> > wrote:
> >> > > I do wonder if the "to disk" threshold should be in the public
> >> > > signature, but don't have offhand a use-case justifying that.
> >> >
> >> > We could, but I figured callers who needed finer-grain control over
> >> > the size of the buffer would use the underlying private API which
> >> > gives them that ability. And as I noted in the code, the choice of
> >> > 100k was completely arbitrary, the result of a lot of squinting and
> >> > hand waving. If somebody has better guestimates (Stefan F.?) as to
> >> > what would be better, feel free to improve upon it.
> >>
> >> FWIW, my point was that the caller (of this now-public API) may be able
> >> to have a better guesstimate as they'd know the use-case, the number of
> >> concurrent spillbuf objects, the typical length (`wc -c`) of the stream,
> >> etc.
> >
> > Right. We always assume "caller knows best", and we try to document our APIs
> > to give them the needed info.
> >
> > I would recommend exposure, with two DEFAULT constants. I'd be -0 with
> > making them 0 and documenting their remapping to "suitable defaults". (ie.
> > allowing callers to use 0 instead of a 20-char symbol)
>
> But some callers don't care; those that do already have a way of
> having finer-grained control: the private API. The blocksize and
You're suggesting a private API as a replacement to a public API.
I'll agree with you, though, if you claim that it's not a problem until
a caller outside of the core libraries has demonstrated a need for this
API. :-)
> maxsize parameters to the spillbuf code are an implementation detail
> of a more generic paradigm, so exposing them through the primary API
> is inappropriate. It's the "generic buffered stream" abstraction that
> callers should be interested in, and letting us improve the
> implementation as we see fit. For callers who want or need the
> specific spillbuf semantics: use the spillbuf-specific private API.
>
> As implemented, using svn_stream_buffered() is no worse than
> unconditionally spooling data to disk (and is in many usecases *much*
> better). Callers get a free benefit now, so let's not worry about
> giving them the additional control at the expense of additional
> complexity until it's shown they need it.
Received on 2012-01-20 14:20:26 CET