[PATCH] D14910: [RuntimeDyld] DenseMap -> std::unordered_map

[Keno Fischer via llvm-commits]

Yes, I'm agreeing with you, which is why I switched it to unordered_map
instead of map as in the original patch. The primary concern here is memory
usage, rather than performance. DenseMap is an absolutely aweful data
structure to use here (perhaps it also has a bug that has it fail to
reclaim memory upon being cleared - I'm not sure). When running the julia
test suite without this patch, this DenseMap consumes 2.5GB (with this
patch the map is at most a handful of MB at peak). Either std::map or
std::unordered_map do fine here.

On Tue, Nov 24, 2015 at 2:06 PM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Tue, Nov 24, 2015 at 10:58 AM, Keno Fischer via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> loladiro retitled this revision from "[RuntimeDyld] DenseMap -> std::map"
>> to "[RuntimeDyld] DenseMap -> std::unordered_map".
>> loladiro updated this revision to Diff 41064.
>> loladiro added a comment.
>>
>> Tested with unordered_map. Seems marginally better in my tests, but
>> probably not statistically significant given my methodology. Still there's
>> no reason the map should be ordered, so might as well use an unordered_map.
>>
>
> But DenseMap isn't ordered either, so this doesn't remove ordering - it
> was never there.
>
> Might be worth having some more numbers before we'd make that change...
>
>
>>
>>
>> Repository:
>>   rL LLVM
>>
>> http://reviews.llvm.org/D14910
>>
>> Files:
>>   lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
>>   lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h
>>
>> Index: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h
>> ===================================================================
>> --- lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h
>> +++ lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h
>> @@ -30,6 +30,7 @@
>>  #include "llvm/Support/SwapByteOrder.h"
>>  #include "llvm/Support/raw_ostream.h"
>>  #include <map>
>> +#include <unordered_map>
>>  #include <system_error>
>>
>>  using namespace llvm;
>> @@ -229,7 +230,7 @@
>>    // Relocations to sections already loaded. Indexed by SectionID which
>> is the
>>    // source of the address. The target where the address will be written
>> is
>>    // SectionID/Offset in the relocation itself.
>> -  DenseMap<unsigned, RelocationList> Relocations;
>> +  std::unordered_map<unsigned, RelocationList> Relocations;
>>
>>    // Relocations to external symbols that are not yet resolved.  Symbols
>> are
>>    // external when they aren't found in the global symbol table of all
>> loaded
>> Index: lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
>> ===================================================================
>> --- lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
>> +++ lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
>> @@ -98,11 +98,11 @@
>>      // The Section here (Sections[i]) refers to the section in which the
>>      // symbol for the relocation is located.  The SectionID in the
>> relocation
>>      // entry provides the section to which the relocation will be
>> applied.
>> -    int Idx = it->getFirst();
>> +    int Idx = it->first;
>>      uint64_t Addr = Sections[Idx].LoadAddress;
>>      DEBUG(dbgs() << "Resolving relocations Section #" << Idx << "\t"
>>                   << format("%p", (uintptr_t)Addr) << "\n");
>> -    resolveRelocationList(it->getSecond(), Addr);
>> +    resolveRelocationList(it->second, Addr);
>>    }
>>    Relocations.clear();
>>
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151124/51cda4ee/attachment.html>