<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Mar 19, 2015 at 5:50 PM, Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
> On 2015-Mar-19, at 16:52, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
><br>
> Totally plausible. Optional/debatable comments:<br>
><br>
> 1) is it worth supporting the more idiomatic clear+shrink_to_fit to be clear of the semantics? ('reset' seems vague) It looks like it's sort of halfway there already, with shrink_and_clear. Could just be generalized a bit more to be part of the solution to this use case<br>
<br>
</span>`reset()` is used with smart pointers to mean "like it's default<br>
constructed", so that's where I was going with it.  I realize this<br>
isn't related to pointers though :).<br>
<br>
I hadn't considered `shrink_to_fit()`, and at first blush I liked it<br>
better than what I chose.<br>
<br>
However, unlike `std::vector<>` where `clear()` is "free",<br>
`SmallPtrSet<>::clear()` is going to `malloc()` a new array.  I guess<br>
another option is to change `clear()` to shrink down to small storage<br>
(a bigger change, but it saves a malloc).  Thoughts on that approach?<br></blockquote><div><br>It seems like totally the right thing - clear() causing allocation sounds pretty bad to me. Lazily wait until there's a use to cause allocation.<br><br>Though the malloc is partly because clear() is semi-smart, trying to shrink a bit (but not too much? Why? perhaps because it expects it'll grow to a similar size, but not quite as big? This seems too smart for its own good, really)<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
If I decide to stick with the current model, do you have any better<br>
names than `reset()`?<br></blockquote><div><br>shrink_to_empty() ? But yeah, clear seems better - and I'd wonder if we should actually break the old clear behavior (maybe check revision history to see if it was introduced for any particular calles that should become clear+shrink_to_fit) - it's not unreasonable for a caller to expect clear() not to deallocate the existing buffer so it can grow again to that size with little/no overhead. <br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> 2) I would perhaps consider testing via looking at the address of an element added after calling reset, rather than using isSmall, but I realize that's totally subjective (& reactive - perhaps if I wrote it, I would've written it as you have)<br>
<br>
</span>Right, right.  I was being lazy here...<br>
<div class="HOEnZb"><div class="h5"><br>
><br>
> On Thu, Mar 19, 2015 at 4:43 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
> 0001 adds `SmallPtrSet::reset()`, which returns `SmallPtrSet` to small<br>
> mode after completely freeing its heap memory.  To facilitate testing,<br>
> I changed `isSmall()` to a public accessor.<br>
><br>
> Thoughts?<br>
><br>
> (0002 is the reason I was looking at this.)<br>
><br>
><br>
><br>
<br>
</div></div></blockquote></div><br></div></div>