[PATCH] D22617: [ELF] - Linkerscript: add InputSectionDescription command to LS parser.

[Eugene Leviant via llvm-commits]

evgeny777 added inline comments.

================
Comment at: ELF/LinkerScript.cpp:220
@@ -211,4 +219,3 @@
 bool LinkerScript<ELFT>::shouldKeep(InputSectionBase<ELFT> *S) {
-  for (StringRef Pat : Opt.KeptSections)
-    if (globMatch(Pat, S->getSectionName()))
-      return true;
+  for (const std::unique_ptr<BaseCommand> &Base : Opt.Commands)
+    if (auto *OutSec = dyn_cast<OutputSectionCommand>(Base.get()))
----------------
grimar wrote:
> evgeny777 wrote:
> > grimar wrote:
> > > evgeny777 wrote:
> > > > It looks like a slow-down compared with previous one, no?
> > > That fits in new design. I am sure we will spend time on optimization of LS later,
> > > when will be able to run benchmarks. And probably many places will need tweaking,
> > > for now I don't think it is critical to change until major slowdown is proven.
> > > 
> > As far as I understand if --gc-sections command line switch is set, then markLive() calls LinkerScript::shouldKeep() for every input section. There could easily be millions of input sections in large projects like WebKit or LLVM itself. It might not be a good idea to sacrifice performance that much in favor of design improvements.
> I think the most slow here is the call of globMatch. Amount of calls remained the same.
Well, the time complexity in your new code is O(I*R), I - number of sections, R - number of rules
Imagine you don't have any kept sections and don't call globMatch. Let's say you have 1 million sections and 10 rules total in script
Your for statements are actually for_each, at average you need about 20 instructions (with -O2 and 100 without optimization) for a single iteration (this is just 'for' statement without body).
Let's say you'll need 50 instructions for each rule (just assumption - can't tell the exact number), so

1 000 000 * 10 * 50 = 500 000 000
On 2 Ghz processor and 1 clock cycle per instruction

t = (5 * 10^8) / (2*10^9) = 0.25 sec.
for doing nothing

Is it worth that?




https://reviews.llvm.org/D22617