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

[Alex Rousskov]

On 06/23/2016 04:49 PM, Nathan Hoad wrote:

> A gentle reminder about this patch, I don't want it to get forgotten :)
> From the history I can see, I don't think there's any work remaining,
> just some more review.

Which was probably true when the first patch iteration was written as
well... I am sorry I could not find the time to review the latest
iteration before it was committed, but I hope it is OK.


Thank you,

Alex.


> On 14 April 2016 at 18:52, Amos Jeffries wrote:
> 
>     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
> 
>     _______________________________________________
>     squid-dev mailing list
>     squid-dev at lists.squid-cache.org <mailto:squid-dev at lists.squid-cache.org>
>     http://lists.squid-cache.org/listinfo/squid-dev
> 
>