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

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo marked 3 inline comments as done.
mstorsjo added a comment.

In D57666#1382661 <https://reviews.llvm.org/D57666#1382661>, @aganea wrote:

> Thanks for fixing this Martin. My mistake, I was annoyed by the fact that the key was sharing its data with the value, went to change the `std::map` into a `std::set`, then I machinally changed the search to `find_if`.


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.

> Is there any way to subsequently add a stress test for this? (ie. the test would fail if `lld-link` takes >5 sec)

Not sure how well it fits into the automated tests, as the exact threshold for what is too long and the input size where it becomes noticeable varies per machine.

I noticed it in a setup where I built code with `ulimit -t 120` (to catch cases where compiler bugs makes it loop infinitely), and even then, the cases I normally build still linked fast - I only noticed it with a larger than usual input.



================
Comment at: COFF/Writer.cpp:1778
+  PartialSectionKey Key = {Name, OutChars};
+  PartialSection *&PSec = PartialSections[Key];
   if (PSec)
----------------
aganea wrote:
> What about `PartialSection *&PSec = PartialSections[{Name, OutChars}];` ?
Sure, I can do that. (I'm a little unfamiliar with the exact dos and don'ts about these, to me, "new" C++11 syntax additions.)


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

https://reviews.llvm.org/D57666