[lld] r260724 - ELF: Remove use of MapVector from LinkerScript.

[Sean Silva via llvm-commits]

On Fri, Feb 12, 2016 at 12:41 PM, Rui Ueyama via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: ruiu
> Date: Fri Feb 12 14:41:43 2016
> New Revision: 260724
>
> URL: http://llvm.org/viewvc/llvm-project?rev=260724&view=rev
> Log:
> ELF: Remove use of MapVector from LinkerScript.
>
> We don't have to use a MapVector here. Instead, just std::vector suffices.
>
> Modified:
>     lld/trunk/ELF/Driver.cpp
>     lld/trunk/ELF/LinkerScript.cpp
>     lld/trunk/ELF/LinkerScript.h
>
> Modified: lld/trunk/ELF/Driver.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Driver.cpp?rev=260724&r1=260723&r2=260724&view=diff
>
> ==============================================================================
> --- lld/trunk/ELF/Driver.cpp (original)
> +++ lld/trunk/ELF/Driver.cpp Fri Feb 12 14:41:43 2016
> @@ -296,7 +296,6 @@ template <class ELFT> void LinkerDriver:
>    SymbolTable<ELFT> Symtab;
>    std::unique_ptr<TargetInfo> TI(createTarget());
>    Target = TI.get();
> -  Script->finalize();
>
>    if (!Config->Shared) {
>      // Add entry symbol.
>
> Modified: lld/trunk/ELF/LinkerScript.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/LinkerScript.cpp?rev=260724&r1=260723&r2=260724&view=diff
>
> ==============================================================================
> --- lld/trunk/ELF/LinkerScript.cpp (original)
> +++ lld/trunk/ELF/LinkerScript.cpp Fri Feb 12 14:41:43 2016
> @@ -28,27 +28,22 @@ using namespace lld::elf2;
>
>  LinkerScript *elf2::Script;
>
> -void LinkerScript::finalize() {
> -  for (const std::pair<StringRef, std::vector<StringRef>> &P : Sections)
> -    for (StringRef S : P.second)
> -      RevSections[S] = P.first;
> -}
> -
>  StringRef LinkerScript::getOutputSection(StringRef S) {
> -  return RevSections.lookup(S);
> +  return Sections.lookup(S);
>  }
>
>  bool LinkerScript::isDiscarded(StringRef S) {
> -  return RevSections.lookup(S) == "/DISCARD/";
> +  return Sections.lookup(S) == "/DISCARD/";
>  }
>
> +// A compartor to sort output sections. Returns -1 or 1 if both
> +// A and B are mentioned in linker scripts. Otherwise, returns 0
> +// to use the default rule which is implemented in Writer.cpp.
>  int LinkerScript::compareSections(StringRef A, StringRef B) {
> -  auto I = Sections.find(A);
> -  auto E = Sections.end();
> -  if (I == E)
> -    return 0;
> -  auto J = Sections.find(B);
> -  if (J == E)
> +  auto E = SectionOrder.end();
> +  auto I = std::find(SectionOrder.begin(), E, A);
> +  auto J = std::find(SectionOrder.begin(), E, B);
> +  if (I == E || J == E)
>      return 0;
>


This is called N log N times while we are sorting. If we are doing O(N)
work in std::find then the sorting is superlinear.
I don't expect the number of output sections to be large, but it seems
wasteful to be using linear search when the MapVector is not really any
more complicated and avoids doing redundant string comparisons.

-- Sean Silva


>    return I < J ? -1 : 1;
>  }
> @@ -349,14 +344,15 @@ void ScriptParser::readSections() {
>  }
>
>  void ScriptParser::readOutputSectionDescription() {
> -  std::vector<StringRef> &V = Script->Sections[next()];
> +  StringRef OutSec = next();
> +  Script->SectionOrder.push_back(OutSec);
>    expect(":");
>    expect("{");
>    while (!Error && !skip("}")) {
>      next(); // Skip input file name.
>      expect("(");
>      while (!Error && !skip(")"))
> -      V.push_back(next());
> +      Script->Sections[next()] = OutSec;
>    }
>  }
>
>
> Modified: lld/trunk/ELF/LinkerScript.h
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/LinkerScript.h?rev=260724&r1=260723&r2=260724&view=diff
>
> ==============================================================================
> --- lld/trunk/ELF/LinkerScript.h (original)
> +++ lld/trunk/ELF/LinkerScript.h Fri Feb 12 14:41:43 2016
> @@ -32,15 +32,14 @@ public:
>    StringRef getOutputSection(StringRef InputSection);
>    bool isDiscarded(StringRef InputSection);
>    int compareSections(StringRef A, StringRef B);
> -  void finalize();
>
>  private:
> -  // Map for SECTIONS command. The key is output section name
> -  // and a value is a list of input section names.
> -  llvm::MapVector<StringRef, std::vector<StringRef>> Sections;
> +  // A map for SECTIONS command. The key is input section name
> +  // and the value is the corresponding output section name.
> +  llvm::DenseMap<StringRef, StringRef> Sections;
>
> -  // Inverse map of Sections.
> -  llvm::DenseMap<StringRef, StringRef> RevSections;
> +  // Output sections are sorted by this order.
> +  std::vector<StringRef> SectionOrder;
>
>    llvm::BumpPtrAllocator Alloc;
>  };
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160212/2aef804d/attachment.html>