<div dir="ltr"><div><div><div>Hello,<br><br></div>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.<br><br></div>Thank you,<br><br></div>Nathan.<br></div><div class="gmail_extra"><br><div class="gmail_quote">On 14 April 2016 at 18:52, Amos Jeffries <span dir="ltr"><<a href="mailto:squid3@treenet.co.nz" target="_blank">squid3@treenet.co.nz</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 14/04/2016 3:56 p.m., Alex Rousskov wrote:<br>
> On 04/13/2016 06:29 PM, Amos Jeffries wrote:<br>
>> On 14/04/2016 4:18 a.m., Alex Rousskov wrote:<br>
>>> On 04/13/2016 08:22 AM, Amos Jeffries wrote:<br>
>>>> I am wondering if the class StoreIOBuffer needs to have move<br>
>>>> constructor/assignment added now that getClientStreamBuffer() is<br>
>>>> returning a temporary variable by-value.<br>
><br>
><br>
>>> Sorry, I do not follow your logic:<br>
>>><br>
>>> 1. AFAIK, move methods are _optimizations_ so any "X needs move<br>
>>> constructor/assignment" statement does not compute for me in a seemingly<br>
>>> non-optimization context like "getClientStreamBuffer() is returning a<br>
>>> temporary variable by-value".<br>
>>><br>
>>> 2. AFAIK, StoreIOBuffer already has move constructor and assignment<br>
>>> operator (provided by the compiler).<br>
><br>
><br>
>> I was wondering if we were paying unnecessary cycles doing the object<br>
>> data copy. If it's provided then thats fine and I'm happy with this if you are.<br>
><br>
> Yes, I believe that we can assume that the compiler provides a move<br>
> constructor for StoreIOBuffer. However, that move constructor does not<br>
> optimize any "object data copy" AFAICT. StoreIOBuffer is essentially a<br>
> POD so the entire object (about 20 bytes) gets "copied" and nothing can<br>
> be "moved" instead, even if we write something by hand. One cannot<br>
> "move" an integer or a raw pointer!<br>
<br>
</span>Well, to be pedantic one can. It means the 'old' object has it set to<br>
0/null afterwards. Its just less than optimal in the POD case.<br>
<br>
If this class were taking a copy of the free functor and releasing the<br>
buffer on destruct it would need to be move instead of copy to prevent<br>
double-free. But thats not happening here.<br>
<span class=""><br>
><br>
> This is a new area for me, but I think that "move" methods optimize<br>
> copying of objects that have non-trivial copy constructors and<br>
> destructors. By definition, PODs do not have those. StoreIOBuffer does<br>
> not either. Am I missing something?<br>
<br>
</span>Not that I can see. Good point.<br>
<span class="HOEnZb"><font color="#888888"><br>
Amos<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
_______________________________________________<br>
squid-dev mailing list<br>
<a href="mailto:squid-dev@lists.squid-cache.org">squid-dev@lists.squid-cache.org</a><br>
<a href="http://lists.squid-cache.org/listinfo/squid-dev" rel="noreferrer" target="_blank">http://lists.squid-cache.org/listinfo/squid-dev</a><br>
</div></div></blockquote></div><br></div>