[squid-dev] [PATCH] SBuf API improvements

[Amos Jeffries]

On 2015-12-31 05:29, Alex Rousskov wrote:
> On 12/30/2015 03:42 AM, Kinkie wrote:
>> reserveSpace(n) is actually just
>> reserveCapacity(n+length()); what about simply getting rid of
>> reserveSpace and rename reserveCapacity to reserve? Minimum surprise
>> here for users here.
> 
> I will use reductio ad absurdum to highlight what I perceive as a flaw
> in your question:
> 
> C++ is just assembly language with a few keywords and curly braces
> added. What about getting rid of those keywords and curly braces and
> just using pure assembly? Minimum surprise for users!
> 

By the considerations I use to judge whether a helper submission is 
suitable assembly is one language that could indeed pass for acceptance.

It would probably fail on the readability, and code safety 
considerations. But that depends on the specific helper design. The 
language itself is portable enough not to be a reason for rejection in 
itself.


> Yes, you can remove protections, but that usually results in a 
> different
> kind of surprises -- the ones you want to avoid the most. I am pretty
> sure that given a statistically significant sample, the amount of buggy
> code calling reserve(length() + n) would be significantly larger than
> the amount of buggy code code calling reserveSpace(n).

Maybe I'm missing something over the last few days discussion, but why 
does reserve(length() + n) imply bugs?
  I was of the understanding that this proposal was to make that the 
right/only way to reserve ?


> 
>> I wouldn't call them "tightly integrated" but simply
>> "bundled". What I'm trying to do is to make them more readable, secure
>> and adaptable first by using c++ in place of the c-with-thin-c++-paint
>> they currently are coded as. Making them more maintainable, hopefully
>> robust and readable is the main goal. In fact, as we can assume that
>> they will be used as reference by others writing their own helpers,
>> it's in everyone's best interests if they are as readable and compact
>> as possible.
> 
> If your goals are readability and adaptability combined with the ease 
> of
> distribution, a scripting language may work [much] better than C++.
> 
> 
>> Let's consider mempools for instance. Having a build-time, static,
>> mechanism which can prevent linking against mempools is IMO tricky and
>> fragile;

Lets not. Except to confirm that the bundled C/C++ helpers are not and 
will not depend on mempools or any other src/ code.

Enforcing that dependency separation is a major reason why I moved the 
helpers/ directory to build before src/ all those years ago in the first 
place.

Further enforcing the split and reducing the remaining build issues is 
part of why I moved the mempools from include/lib to src/mem/ more 
recently.

> 
> It is difficult to argue efficiently with fuzzy characteristics like
> "tricky" and "fragile", but I think that making SBuf available without
> mempools is better than making SBuf-driven algorithms generic. The
> alternatives (such as generics) are not necessarily trickier or more
> fragile, but nevertheless worse overall.
> 

Nod. If anyone is looking at using heavyweight components of Squid (like 
mempools, SBuf etc) in the helpers then it is probably a situation where 
the helper is not a helper but a new mode of operation for Squid 
workers.

For example; Diskers ride that line of distinction - having a relatively 
small scope of operations (Store) but that scope is a heavyweight 
component. As compared to the diskd helper which does similar tasks but 
with simpler components.


> 
>> Let me restrict the scope to "the C-ish and C++ helpers we bundle"
> 
> It could have been worse. Imagine us bundling helpers written in
> assembly. After you reject that image as obviously absurd, I hope you
> will notice a funny aftertaste questions lingering for a few seconds:
> Why are we bundling *C++* helpers? If we continue to bundle C++ 
> helpers,
> what does it say about our true goals with regard to bundled helper
> qualities? Are we after ease of writing? Ease of reading? Ease of
> adapting? Ease of maintaining? Ease of bundling? Performance?
> 
> 
>>> 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.
>> 
>> There are. However:
> 
> Please note that I was only talking about "libraries and modules" to 
> use
> with _helpers_, not with Squid proper.
> 
> 
>> - the ones I could find seem to be quite generic, we have different
>> needs in different contexts
> 
> This item is too general to agree or disagree with.
> 
> 
>> - impedence mismatch. These are either C or use std::string; this
>> generally means two more data-copies if we wish to use SBuf (or
>> std::string with a custom allocator)
> 
> I lost you here. If helpers wish to use SBuf, then they do not need 3rd
> party libraries and Squid does not need to make its SBuf algorithms
> generic. If helpers do not wish to use SBuf, then 3rd party libraries 
> do
> not introduce any extra copies (from SBufs).
> 

Nod. IMO, the need for a third-party RFC1738/3986 coder library is 
primarily for the helpers, not Squid itself. We already have this 
adequate coder logic for Squid internals. Moving the helpers to a 
third-party library we can then adapt our custom logic for better 
suiting SBuf etc.

This both helps with simplifying the helpers dependency on Squid code 
and eases third-party helpers being written as they no longer have to 
cut-n-paste our rfc1738 code.

So, if you found a FOSS library that provides RFC1738/3986 encoding and 
supports std:string please look at adopting it for the helpers/ code.


> 
>> - finding one which is generally available on all OSes we care about
>> seems not trivial.
> 
> Agreed. We can add portability or ease of deployment to the bundled
> helper criteria. It is a topic on its own!
> 

Portability is already one of the main criteria for bundled helpers. But 
just important, not critical. What is critical is being able to detect 
the OS environments where they will fail to execute or build and avoid 
auto-enabling them.

> 
>>> 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.
> 
> 
>> Could be, but we'd then need to redesign mempools to be stub-able, or
>> templatize SBuf with an allocator and isolate it from mempools (I
>> focus on these as these are the dependency bringing in most baggage.
> 
> I hope there are better ways to get B because, IIRC, SBufs do not know
> [much] about mempools. I am oversimplifying, but I suspect that
> separating SBufs from needless hard-coded dependencies requires not 
> much
> more than providing a memAllocString() and memFreeString() function
> implementations that do not depend on memory pools. After a bit of
> cleanup, it would be just a matter of linking with the right object 
> file
> (Squid gets trueMem.o while helpers get fakeMem.o) or internal library.
> 
> 
>> Given the fact that we won't probably require many common algorithms,
>> I believe that A is not a bad an option as you do.
> 
> Glad you at least agree that A is bad. I know that is not enough to 
> stop
> you from implementing it, but I still hope that you will try to do B
> instead.

Both A and B are bad.

I prefer C; ensuring helper code is a simple as possible with as few 
dependencies - *particularly* from the complex code within src/.


IMHO, our primary goal with these helpers is to provide good quality 
demo code for others to use as template for writing their own. With 
secondary goal of acting as a source of some ready-made helpers for 
common installation needs.

That places code readability (not quite quality), stand-alone 
simplicity, and performance/speed fir the first goal above language or 
portability. Though the latter two are still high importance as they 
pertain to the second goal.

The C++ nature of the helpers is due to our personal preferences for 
coding that way (more experienced dev attention and code quality etc) 
combined with the bundling providing a guarantee of C++ compiler being 
available (portability), nothing more.

Amos