<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Feb 23, 2015 at 9:43 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Mon, Feb 23, 2015 at 9:32 AM, Shankar Easwaran <span dir="ltr"><<a href="mailto:shankare@codeaurora.org" target="_blank">shankare@codeaurora.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>On 2/23/2015 10:56 AM, David Blaikie wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Mon, Feb 23, 2015 at 5:50 AM, Shankar Easwaran <<a href="mailto:shankare@codeaurora.org" target="_blank">shankare@codeaurora.org</a>><br>
wrote:<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: shankare<br>
Date: Mon Feb 23 07:50:23 2015<br>
New Revision: 230219<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=230219&view=rev" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project?rev=230219&view=rev</a><br>
Log:<br>
[ELF][Writer] Use llvm::StringMap instead<br>
<br>
</blockquote>
This does change the semantics somewhat - StringMaps own their strings. do<br>
you need that? (if so, is there a test case for where the map<StringRef was<br>
insufficient?) Or does this use case neither require nor forbid copying the<br>
strings, and just benefit from the StringMap optimizations in memory, etc?<br>
<br>
</blockquote></span>
Thanks for the review!.<br>
<br>
Oh, I dont want StringMaps to start copying the strings and own it. Is the alternative to use llvm::DenseMap then ?<br>
<br>
The reason I dont want to use std::map was for performance.</blockquote></span><div><br>If it's performance, it's possible that copying the strings is still better than not - your perf experiments/numbers should show if it's the wrong tradeoff.<br><br>But yeah, if you're running some experiments, it might be worth trying a DenseMap to see if non-owning StringRefs are the better tradeoff.<br></div></div></div></div></blockquote><div><br>Oh, and if it turns out the perf tradeoff is in favor of StringMap, it might be worth a comment mentioning that the ownership isn't needed in this use case (in case someone wants to refactor this again later).<br><br>- David</div></div></div></div>