<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 29, 2016 at 5:53 PM, Hal Finkel via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">hfinkel added inline comments.<br>
<span class=""><br>
================<br>
Comment at: include/llvm/ADT/StringMap.h:261<br>
@@ +260,3 @@<br>
</span><span class="">+    NumItems = RHS.NumItems;<br>
+    NumTombstones = RHS.NumTombstones;<br>
+    for (unsigned I = 0, E = NumBuckets; I != E; ++I) {<br>
</span>----------------<br>
<span class="">joker.eph wrote:<br>
> hfinkel wrote:<br>
> > joker.eph wrote:<br>
> > > I wonder if we really want to keep the tombstones or if it wouldn't be better to reinsert the elements?<br>
> > I don't have a good feeling for this; it seems like this could mean one of two things:<br>
> ><br>
> >  1. We start with the same number of buckets as the RHS; in this case keeping the tombstones vs. not just affects whether we write a 0 or -1 into the pointer array for the empty slot (although insertion will prefer that slot over further probing).<br>
> >  2. Start with a minimal number of buckets. Potentially more memory efficient, but essentially leads to "rehashing" (which in this case means re-probing for all inputs).<br>
> ><br>
> > What do you prefer?<br>
> It seems to me that when looking for an entry in the map, the search stops when it find an empty slot, but continue when there is a tombstone. If I understand your 1), it is just replacing tombstones with empty buckets, but that would break the search for existing entries that are after the tombstones.<br>
><br>
> The 2) is basically what I was thinking of, eventually allocating the same number of buckets, but indeed reinserting each entry (reusing the existing known hash). Not that I have a strong preference, I was rather wondering if you considered this and what is the rational for one or the other (and if there is a reason, I'd rather have it documented in the code for future reference).<br>
> I hope David could chime in with an opinion.<br>
> It seems to me that when looking for an entry in the map, the search stops when it find an empty slot, but continue when there is a tombstone. If I understand your 1), it is just replacing tombstones with empty buckets, but that would break the search for existing entries that are after the tombstones.<br>
<br>
</span>I believe you're correct; scratch that option.<br>
<span class=""><br>
> The 2) is basically what I was thinking of, eventually allocating the same number of buckets, but indeed reinserting each entry (reusing the existing known hash). Not that I have a strong preference, I was rather wondering if you considered this and what is the rational for one or the other (and if there is a reason, I'd rather have it documented in the code for future reference).<br>
<br>
</span>My thought was the following: Most StringMaps don't have many tombstones, because many use cases don't ever (or only rarely) delete things from their string maps. As a result, just copying the structure as-is (any tombstones included) will be faster than re-probing for all keys with little downside.<br></blockquote><div><br></div><div>Data might be nice, if you happen to have a way to gather some easily at hand.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="im HOEnZb"><br>
> I hope David could chime in with an opinion.<br>
<br>
<br>
<br>
</span><div class="HOEnZb"><div class="h5"><a href="http://reviews.llvm.org/D18506" rel="noreferrer" target="_blank">http://reviews.llvm.org/D18506</a><br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div></div>