[COFF] Deduplicate Export before parsing

[takuto.ikuta]

Summary

This patch reduces lld link time of chromium's blink_core.dll in
component build.

Total size of input argument in .directives become nearly 300MB in the
build and almost all argument is /EXPORT.

This CL deduplicates exported symbol before parsing.
Original idea of this deduplication for Export is suggested by Rui Ueyama.

On my desktop machine, 4 times stats of the link time are like below.
Improved around 20%.

This patch

TotalSeconds : 10.2710421
TotalSeconds : 9.950322
TotalSeconds : 10.3146625
TotalSeconds : 10.437696
Avg : 10.24343065

master

TotalSeconds : 13.1020911
TotalSeconds : 13.1314426
TotalSeconds : 13.0887463
TotalSeconds : 13.1096681
Avg : 13.107987025

[rnk]

I think this is fine, but we should refactor export handling. We already do a lot of work like this in fixupExports, and it would be better if we could move the logic closer together.

lld/COFF/Driver.cpp

1050

StringRef is a value type, it's better to just say StringRef RawE and drop the const &.

1051

Indentation needs fixing.

[takuto.ikuta]

Fix StringRef type and indent

[takuto.ikuta]

Thank you for review.
I do not have confidence the way to do good refactoring for export handling.
Can I leave FIXME instead?

[ruiu]

lld/COFF/Config.h

109

I think "DirectivesExports" is too narrowly scoped. It should not only contain /EXPORT options appeared in the directives section but also contain /EXPORT options in the command line, because distinguishing the two doesn't make sense. They should be handled in exactly the same way.

lld/COFF/Driver.cpp

1050

I don't like to leave a FIXME comment in the code because in many cases it won't be fixed and just distract people from understanding the code. If you need to fix something, please fix it now.

[takuto.ikuta]

Merged parsing code.

[ruiu]

lld/COFF/Config.h

109

I'd name this ExportStrings, as RawExports sounds like a new feature or something to me.

lld/COFF/Driver.cpp

1122

I don't think this is correct. You are processing Config->RawExports too early. After this piece of code is executed, there is still a chance that a new file is added to the input file list, and its directive sections won't be processed correctly.

[ruiu]

I wonder something like this https://reviews.llvm.org/D41607 would work.

[takuto.ikuta]

Rui-san improved performance with better implementaion in https://reviews.llvm.org/D41607
I close this.

lld/COFF/Config.h

 //===- Config.h -------------------------------------------------*- C++ -*-===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLD_COFF_CONFIG_H
 #define LLD_COFF_CONFIG_H
 
+#include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Support/CachePruning.h"
 #include <cstdint>
 #include <map>
 #include <set>
 #include <string>
 
struct Configuration {
   std::vector<Symbol *> GCRoot;
 
   std::set<StringRef> NoDefaultLibs;
   bool NoDefaultLibAll = false;
 
   // True if we are creating a DLL.
   bool DLL = false;
   StringRef Implib;
+
+  // Export in directive sections, parsed later.
+  llvm::SetVector<StringRef> DirectiveExports;
+
   std::vector<Export> Exports;
   std::set<std::string> DelayLoads;
   std::map<std::string, int> DLLOrder;
   Symbol *DelayLoadHelper = nullptr;
 
   bool SaveTemps = false;
 
   // Used for SafeSEH.

lld/COFF/Driver.cpp

for (auto *Arg : Args) {
     case OPT_defaultlib:
       if (Optional<StringRef> Path = findLib(Arg->getValue()))
         enqueuePath(*Path, false);
       break;
     case OPT_entry:
       Config->Entry = addUndefined(mangle(Arg->getValue()));
       break;
     case OPT_export: {
-      Export E = parseExport(Arg->getValue());
-      if (Config->Machine == I386 && Config->MinGW) {
-        if (!isDecorated(E.Name))
-          E.Name = Saver.save("_" + E.Name);
-        if (!E.ExtName.empty() && !isDecorated(E.ExtName))
-          E.ExtName = Saver.save("_" + E.ExtName);
-      }
-      E.Directives = true;
-      Config->Exports.push_back(E);
+      Config->DirectiveExports.insert(Arg->getValue());
       break;
     }
     case OPT_failifmismatch:
       checkFailIfMismatch(Arg->getValue());
       break;
     case OPT_incl:
       addUndefined(Arg->getValue());
       break;
void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
 
   // Windows specific -- Create a resource file containing a manifest file.
   if (Config->Manifest == Configuration::Embed)
     addBuffer(createManifestRes(), false);
 
   // Read all input files given via the command line.
   run();
 
+  // Parse deduplicated Exports in directive sections.
+  // FIXME: fixupExports() does also deduplication, move this code closer to
+  //        the function.
+  for (StringRef RawE : Config->DirectiveExports) {
+    Export E = parseExport(RawE);
+    if (Config->Machine == I386 && Config->MinGW) {
+      if (!isDecorated(E.Name))
+        E.Name = Saver.save("_" + E.Name);
+      if (!E.ExtName.empty() && !isDecorated(E.ExtName))
+        E.ExtName = Saver.save("_" + E.ExtName);
+    }
+    E.Directives = true;
+    Config->Exports.push_back(E);
+  }
+
   // We should have inferred a machine type by now from the input files, but if
   // not we assume x64.
   if (Config->Machine == IMAGE_FILE_MACHINE_UNKNOWN) {
     warn("/machine is not specified. x64 is assumed");
     Config->Machine = AMD64;
   }
 
   // Input files can be Windows resource files (.res files). We use
void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
 
   // Handle /export
   for (auto *Arg : Args.filtered(OPT_export)) {
     Export E = parseExport(Arg->getValue());
     if (Config->Machine == I386) {
       if (!isDecorated(E.Name))
         E.Name = Saver.save("_" + E.Name);
       if (!E.ExtName.empty() && !isDecorated(E.ExtName))
         E.ExtName = Saver.save("_" + E.ExtName);
     }
     Config->Exports.push_back(E);
   }
 
   // Handle /def
   if (auto *Arg = Args.getLastArg(OPT_deffile)) {
     // parseModuleDefs mutates Config object.
     parseModuleDefs(Arg->getValue());