[lld] r230219 - [ELF][Writer] Use llvm::StringMap instead

David Blaikie dblaikie at gmail.com
Mon Feb 23 09:44:55 PST 2015


On Mon, Feb 23, 2015 at 9:43 AM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Mon, Feb 23, 2015 at 9:32 AM, Shankar Easwaran <shankare at codeaurora.org
> > wrote:
>
>> On 2/23/2015 10:56 AM, David Blaikie wrote:
>>
>>> On Mon, Feb 23, 2015 at 5:50 AM, Shankar Easwaran <
>>> shankare at codeaurora.org>
>>> wrote:
>>>
>>>  Author: shankare
>>>> Date: Mon Feb 23 07:50:23 2015
>>>> New Revision: 230219
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=230219&view=rev
>>>> Log:
>>>> [ELF][Writer] Use llvm::StringMap instead
>>>>
>>>>  This does change the semantics somewhat - StringMaps own their
>>> strings. do
>>> you need that? (if so, is there a test case for where the map<StringRef
>>> was
>>> insufficient?) Or does this use case neither require nor forbid copying
>>> the
>>> strings, and just benefit from the StringMap optimizations in memory,
>>> etc?
>>>
>>>  Thanks for the review!.
>>
>> Oh, I dont want StringMaps to start copying the strings and own it. Is
>> the alternative to use llvm::DenseMap then ?
>>
>> The reason I dont want to use std::map was for performance.
>
>
> 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.
>
> 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.
>

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).

- David
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150223/429f6aeb/attachment.html>


More information about the llvm-commits mailing list