<div dir="ltr">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.</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Nov 24, 2015 at 2:06 PM, 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 Tue, Nov 24, 2015 at 10:58 AM, Keno Fischer 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">loladiro retitled this revision from "[RuntimeDyld] DenseMap -> std::map" to "[RuntimeDyld] DenseMap -> std::unordered_map".<br>
loladiro updated this revision to Diff 41064.<br>
loladiro added a comment.<br>
<br>
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.<br></blockquote><div><br></div></span><div>But DenseMap isn't ordered either, so this doesn't remove ordering - it was never there.<br><br>Might be worth having some more numbers before we'd make that change... </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
<br>
<br>
Repository:<br>
rL LLVM<br>
<br>
<a href="http://reviews.llvm.org/D14910" rel="noreferrer" target="_blank">http://reviews.llvm.org/D14910</a><br>
<br>
Files:<br>
lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp<br>
lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h<br>
<br>
Index: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h<br>
===================================================================<br>
--- lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h<br>
+++ lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h<br>
@@ -30,6 +30,7 @@<br>
#include "llvm/Support/SwapByteOrder.h"<br>
#include "llvm/Support/raw_ostream.h"<br>
#include <map><br>
+#include <unordered_map><br>
#include <system_error><br>
<br>
using namespace llvm;<br>
@@ -229,7 +230,7 @@<br>
// Relocations to sections already loaded. Indexed by SectionID which is the<br>
// source of the address. The target where the address will be written is<br>
// SectionID/Offset in the relocation itself.<br>
- DenseMap<unsigned, RelocationList> Relocations;<br>
+ std::unordered_map<unsigned, RelocationList> Relocations;<br>
<br>
// Relocations to external symbols that are not yet resolved. Symbols are<br>
// external when they aren't found in the global symbol table of all loaded<br>
Index: lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp<br>
===================================================================<br>
--- lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp<br>
+++ lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp<br>
@@ -98,11 +98,11 @@<br>
// The Section here (Sections[i]) refers to the section in which the<br>
// symbol for the relocation is located. The SectionID in the relocation<br>
// entry provides the section to which the relocation will be applied.<br>
- int Idx = it->getFirst();<br>
+ int Idx = it->first;<br>
uint64_t Addr = Sections[Idx].LoadAddress;<br>
DEBUG(dbgs() << "Resolving relocations Section #" << Idx << "\t"<br>
<< format("%p", (uintptr_t)Addr) << "\n");<br>
- resolveRelocationList(it->getSecond(), Addr);<br>
+ resolveRelocationList(it->second, Addr);<br>
}<br>
Relocations.clear();<br>
<br>
<br>
<br>
<br></div></div>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">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>
<br></blockquote></div><br></div></div>
</blockquote></div><br></div>