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 > [email protected] <mailto:[email protected]> > http://lists.squid-cache.org/listinfo/squid-dev > > _______________________________________________ squid-dev mailing list [email protected] http://lists.squid-cache.org/listinfo/squid-dev
