[lld] 01b5f52 - [COFF] Add a fastpath for /INCLUDE: in .drective sections

[Reid Kleckner via llvm-commits]

Author: Reid Kleckner
Date: 2020-04-28T10:35:57-07:00
New Revision: 01b5f521408d943dcb05455c5168ae19bcfaa98a

URL: https://github.com/llvm/llvm-project/commit/01b5f521408d943dcb05455c5168ae19bcfaa98a
DIFF: https://github.com/llvm/llvm-project/commit/01b5f521408d943dcb05455c5168ae19bcfaa98a.diff

LOG: [COFF] Add a fastpath for /INCLUDE: in .drective sections

This speeds up linking chrome.dll with PGO instrumentation by 13%
(154271ms -> 134033ms).

LLVM's Option library is very slow. In particular, it allocates at least
one large-ish heap object (Arg) for every argument. When PGO
instrumentation is enabled, all the __profd_* symbols are added to the
@llvm.used list, which compiles down to these /INCLUDE: directives. This
means we have O(#symbols) directives to parse in the section, so we end
up allocating an Arg for every function symbol in the object file. This
is unnecessary.

To address the issue and speed up the link, extend the fast path that we
already have for /EXPORT:, which has similar scaling issues.

I promise that I took a hard look at optimizing the Option library, but
its data structures are very general and would need a lot of cleanup. We
have accumulated lots of optional features (option groups, aliases,
multiple values) over the years, and these are now properties of every
parsed argument, when the vast majority of arguments do not use these
features.

Reviewed By: thakis

Differential Revision: https://reviews.llvm.org/D78845

Added: 
    

Modified: 
    lld/COFF/Driver.cpp
    lld/COFF/Driver.h
    lld/COFF/DriverUtils.cpp

Removed: 
    


################################################################################
diff  --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 08415fa00984..c2bb0db6e6ae 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -343,11 +343,9 @@ void LinkerDriver::parseDirectives(InputFile *file) {
   ArgParser parser;
   // .drectve is always tokenized using Windows shell rules.
   // /EXPORT: option can appear too many times, processing in fastpath.
-  opt::InputArgList args;
-  std::vector<StringRef> exports;
-  std::tie(args, exports) = parser.parseDirectives(s);
+  ParsedDirectives directives = parser.parseDirectives(s);
 
-  for (StringRef e : exports) {
+  for (StringRef e : directives.exports) {
     // If a common header file contains dllexported function
     // declarations, many object files may end up with having the
     // same /EXPORT options. In order to save cost of parsing them,
@@ -366,7 +364,11 @@ void LinkerDriver::parseDirectives(InputFile *file) {
     config->exports.push_back(exp);
   }
 
-  for (auto *arg : args) {
+  // Handle /include: in bulk.
+  for (StringRef inc : directives.includes)
+    addUndefined(inc);
+
+  for (auto *arg : directives.args) {
     switch (arg->getOption().getID()) {
     case OPT_aligncomm:
       parseAligncomm(arg->getValue());

diff  --git a/lld/COFF/Driver.h b/lld/COFF/Driver.h
index cc2f25a6f95e..4b6d0c9dcbe3 100644
--- a/lld/COFF/Driver.h
+++ b/lld/COFF/Driver.h
@@ -41,6 +41,17 @@ class COFFOptTable : public llvm::opt::OptTable {
   COFFOptTable();
 };
 
+// The result of parsing the .drective section. The /export: and /include:
+// options are handled separately because they reference symbols, and the number
+// of symbols can be quite large. The LLVM Option library will perform at least
+// one memory allocation per argument, and that is prohibitively slow for
+// parsing directives.
+struct ParsedDirectives {
+  std::vector<StringRef> exports;
+  std::vector<StringRef> includes;
+  llvm::opt::InputArgList args;
+};
+
 class ArgParser {
 public:
   // Parses command line options.
@@ -52,8 +63,7 @@ class ArgParser {
   // Tokenizes a given string and then parses as command line options in
   // .drectve section. /EXPORT options are returned in second element
   // to be processed in fastpath.
-  std::pair<llvm::opt::InputArgList, std::vector<StringRef>>
-  parseDirectives(StringRef s);
+  ParsedDirectives parseDirectives(StringRef s);
 
 private:
   // Concatenate LINK environment variable.

diff  --git a/lld/COFF/DriverUtils.cpp b/lld/COFF/DriverUtils.cpp
index f1b50e6142e1..5c6210a0c27a 100644
--- a/lld/COFF/DriverUtils.cpp
+++ b/lld/COFF/DriverUtils.cpp
@@ -862,14 +862,16 @@ opt::InputArgList ArgParser::parse(ArrayRef<const char *> argv) {
 
 // Tokenizes and parses a given string as command line in .drective section.
 // /EXPORT options are processed in fastpath.
-std::pair<opt::InputArgList, std::vector<StringRef>>
-ArgParser::parseDirectives(StringRef s) {
-  std::vector<StringRef> exports;
+ParsedDirectives ArgParser::parseDirectives(StringRef s) {
+  ParsedDirectives result;
   SmallVector<const char *, 16> rest;
 
   for (StringRef tok : tokenize(s)) {
     if (tok.startswith_lower("/export:") || tok.startswith_lower("-export:"))
-      exports.push_back(tok.substr(strlen("/export:")));
+      result.exports.push_back(tok.substr(strlen("/export:")));
+    else if (tok.startswith_lower("/include:") ||
+             tok.startswith_lower("-include:"))
+      result.includes.push_back(tok.substr(strlen("/include:")));
     else
       rest.push_back(tok.data());
   }
@@ -878,13 +880,13 @@ ArgParser::parseDirectives(StringRef s) {
   unsigned missingIndex;
   unsigned missingCount;
 
-  opt::InputArgList args = table.ParseArgs(rest, missingIndex, missingCount);
+  result.args = table.ParseArgs(rest, missingIndex, missingCount);
 
   if (missingCount)
-    fatal(Twine(args.getArgString(missingIndex)) + ": missing argument");
-  for (auto *arg : args.filtered(OPT_UNKNOWN))
-    warn("ignoring unknown argument: " + arg->getAsString(args));
-  return {std::move(args), std::move(exports)};
+    fatal(Twine(result.args.getArgString(missingIndex)) + ": missing argument");
+  for (auto *arg : result.args.filtered(OPT_UNKNOWN))
+    warn("ignoring unknown argument: " + arg->getAsString(result.args));
+  return result;
 }
 
 // link.exe has an interesting feature. If LINK or _LINK_ environment