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 <gst...@gmail.com> wrote: > > > > On Jan 19, 2012 11:02 PM, "Daniel Shahaf" <danie...@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 <danie...@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.