[squid-dev] [PATCH] Increase request buffer size to 64kb

[Amos Jeffries]

On 14/04/2016 3:56 p.m., Alex Rousskov wrote:
> On 04/13/2016 06:29 PM, Amos Jeffries wrote:
>> On 14/04/2016 4:18 a.m., Alex Rousskov wrote:
>>> On 04/13/2016 08:22 AM, Amos Jeffries wrote:
>>>> I am wondering if the class StoreIOBuffer needs to have move
>>>> constructor/assignment added now that getClientStreamBuffer() is
>>>> returning a temporary variable by-value.
> 
> 
>>> Sorry, I do not follow your logic:
>>>
>>> 1. AFAIK, move methods are _optimizations_ so any "X needs move
>>> constructor/assignment" statement does not compute for me in a seemingly
>>> non-optimization context like "getClientStreamBuffer() is returning a
>>> temporary variable by-value".
>>>
>>> 2. AFAIK, StoreIOBuffer already has move constructor and assignment
>>> operator (provided by the compiler).
> 
> 
>> I was wondering if we were paying unnecessary cycles doing the object
>> data copy. If it's provided then thats fine and I'm happy with this if you are.
> 
> Yes, I believe that we can assume that the compiler provides a move
> constructor for StoreIOBuffer. However, that move constructor does not
> optimize any "object data copy" AFAICT. StoreIOBuffer is essentially a
> POD so the entire object (about 20 bytes) gets "copied" and nothing can
> be "moved" instead, even if we write something by hand. One cannot
> "move" an integer or a raw pointer!

Well, to be pedantic one can. It means the 'old' object has it set to
0/null afterwards. Its just less than optimal in the POD case.

If this class were taking a copy of the free functor and releasing the
buffer on destruct it would need to be move instead of copy to prevent
double-free. But thats not happening here.

> 
> This is a new area for me, but I think that "move" methods optimize
> copying of objects that have non-trivial copy constructors and
> destructors. By definition, PODs do not have those. StoreIOBuffer does
> not either. Am I missing something?

Not that I can see. Good point.

Amos