From 01b5f521408d943dcb05455c5168ae19bcfaa98a Mon Sep 17 00:00:00 2001 From: Reid Kleckner Date: Fri, 24 Apr 2020 17:26:17 -0700 Subject: [PATCH] [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 --- lld/COFF/Driver.cpp | 12 +++++++----- lld/COFF/Driver.h | 14 ++++++++++++-- lld/COFF/DriverUtils.cpp | 20 +++++++++++--------- 3 files changed, 30 insertions(+), 16 deletions(-) diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp index 08415fa..c2bb0db 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 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 cc2f25a..4b6d0c9 100644 --- a/lld/COFF/Driver.h +++ b/lld/COFF/Driver.h @@ -41,6 +41,17 @@ public: 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 exports; + std::vector includes; + llvm::opt::InputArgList args; +}; + class ArgParser { public: // Parses command line options. @@ -52,8 +63,7 @@ public: // 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> - 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 f1b50e6..5c6210a 100644 --- a/lld/COFF/DriverUtils.cpp +++ b/lld/COFF/DriverUtils.cpp @@ -862,14 +862,16 @@ opt::InputArgList ArgParser::parse(ArrayRef argv) { // Tokenizes and parses a given string as command line in .drective section. // /EXPORT options are processed in fastpath. -std::pair> -ArgParser::parseDirectives(StringRef s) { - std::vector exports; +ParsedDirectives ArgParser::parseDirectives(StringRef s) { + ParsedDirectives result; SmallVector 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 -- 2.7.4