[squid-dev] [PATCH] SBuf API improvements

[Alex Rousskov]

On 12/29/2015 02:59 PM, Kinkie wrote:
> On Mon, Dec 28, 2015 at 10:27 PM, Alex Rousskov wrote:
>> On 12/28/2015 06:41 AM, Kinkie wrote:
>>> It is debateable whether reserveCapacity should be just renamed
>>> instead of aliased; feedback is welcome.

>> I do not have a strong opinion on this. reserveCapacity() is not called
>> reserve() today because we also have reserveSpace() and knowing that
>> there are two different methods is important for some callers. Once you
>> add plain reserve(), the probability that a caller will notice
>> reserveSpace() that the caller needs will go down.

> We could leave both and mark in the documentation that it's meant for
> API compatibility and should not be used by code calling SBuf. what do
> you think?

Having three methods (reserve, reserveCapacity, and reserveSpace) is a
poor solution to the problem that the two methods (reserveCapacity and
reserveSpace) were solving. Yes, adding a not-to-be-used third name
might be better than renaming the existing one, but, again, the overall
direction of your changes eliminates good options, forcing you to pick
among the bad ones.


>>> The objective of this change will be clear in the followup rfc3986
>>> patch, which is templatized in order to be available both to helpers
>>> who use std::string and to squid using SBuf.

>> SBuf is already suffering from being too many things to too many
>> different callers. I suspect that requiring API compatibility with
>> std::string [in certain poorly defined areas] will make the situation
>> worse, but I cannot prove that. Same for writing templated code that can
>> work with both std::string and SBuf.
>>
>> If helpers really need SBuf, give them SBuf. If helpers want to use
>> std::string, they already can. Assuring that one can be substituted with
>> another feels like a false goal that would bring more trouble in the
>> long run than it would help in the short term. Again, I cannot prove
>> that and will not veto these changes.


> helpers do not need SBuf, in fact they don't want it, they need
> percent-encoding and a c++ string management system.

I doubt that all helpers are the same with regard to string needs. Many
helpers do not need C++ at all. I assume that you are thinking about
helpers maintained by the Squid Project and helpers that we decided to
make very efficient and tightly integrated with Squid code.


> The main benefit of SBuf over std::string is its integration with
> support infrastructure in squid proper (mempools and cachemgr), which
> is definitely too much to add to helpers as well. We'd end up filling
> them with stubs which would make maintaining the build system
> complicated - see how much effort it is to do so for unit tests.

I certainly agree that it is easier to add a few methods to SBuf and
craft a templated encoding loop than it is to make SBufs independent
from memory pools [and cache manager]. The ease of a small change does
not necessarily make it the best long-term solution though. There are
other factors to consider.

You use existing unit test build difficulties as an argument to avoid
using SBufs in helpers [because helpers will face similar difficulties].
That argument is valid _if_ those build difficulties are inherent in the
SBuf concept. However, if those difficulties are simply SBuf
implementation bugs/deficiencies, then it would be better to address
those SBuf deficiencies than to work around [and exacerbate!] them in
one more place.


> So IMO helpers need std::string, 

and Perl $string, and Javascript String. There are many different
helpers with very different needs...


> and templated code is the best
> solution I could come up with to avoid having to maintain three
> different implementations of percent-encoding. Suggestions on
> alternate paths to obtain the same objective are of course welcome.

Since Squid is (or should be) using standard encodings, I suspect there
are libraries and modules (for various programming languages) that
already provide appropriate encoding support. We would not have to
maintain any of those implementations.

However, let's assume that our ultimate goal[1] is to ship fast,
production-quality helpers that are built without external dependencies
(i.e., built using Squid libraries) and that Squid code is (or will be)
using SBuf for most string and buffering operations.

Quality helpers are likely to need a variety of string and buffering
operations. Which of the following approaches is better long-term?

A. Support a growing number of templated algorithms while making SBuf
API more and more interchangeable with std::string (while at the same
time providing a growing number of SBuf methods for raw buffer operations).

B. Make SBufs and SBuf-driven algorithms easily available in helpers by
making SBuf storage/memory backing (and any other current dependencies
on Squid core) easily replaceable with something that does not need any
integration with Squid core.

To me, option B looks like the right overall direction.

------------

[1] It is a dangerous and questionable assumption: Why use something so
inefficient and awkward as the helper API if Squid-native code and
performance are the primary criteria?!

Alex.