[PATCH] D57666: [LLD] [COFF] Avoid O(n^2) insertion into PartialSections

[Alexandre Ganea via Phabricator via llvm-commits]

aganea added a comment.

> ! In D57666#1382703 <https://reviews.llvm.org/D57666#1382703>, @mstorsjo wrote:
>  Yeah, it's a bit inelegant to have it duplicated. I considered removing it from the value part here as well, but that would be more or less a revert of your commit I presume.

No, I would need the full value to back reference from `OutputSection` (see D54802 <https://reviews.llvm.org/D54802>)



================
Comment at: COFF/Writer.cpp:237
   std::unique_ptr<FileOutputBuffer> &Buffer;
-  std::set<PartialSection *, PartialLess> PartialSections;
+  std::map<PartialSectionKey, PartialSection *> PartialSections;
   std::vector<OutputSection *> OutputSections;
----------------
On a second thought, and if we wanted to be optimal, I was wondering if we shouldn't be using a `llvm::DenseSet` here instead. `std::map` isn't known to scale particularly well with many elements, and you're at the implementation's mercy. At least the behavior will be more consistent with an internal structure.

I did use a `DenseMap` initially, but that requires more plumbing, because you need to retain the insertion order ( for later, when emitting data into the PDB). I did not see the advantage at the time, given that MSVC does not generate that many sections, but in your case, it matters. What do you think?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57666/new/

https://reviews.llvm.org/D57666