[lld] r314644 - [ICF] Include section contents in section hash values.

[Rui Ueyama via llvm-commits]

If an executable consists of a lot of input sections that are obviously
different without even seeing the section contents, computing hash values
early is a waste of time. But looks like it is not the case.

I can think of a few reasons: (1) without a content hash, we sometimes have
to compare contents of the same section many times, (2) the ICF algorithm
works better if it starts with a large number of classes (i.e. smaller # of
sections / # of classes ratio), (3) computing a hash value is pretty fast
anyway. But they are just my guesses.

On Mon, Oct 2, 2017 at 10:24 AM, Sean Silva <chisophugis at gmail.com> wrote:

> We're there ever times previously where we would avoid reading the section
> contents due to delaying the content hash?
>
> -- Sean Silva
>
> On Oct 1, 2017 6:22 PM, "Rui Ueyama via llvm-commits" <
> llvm-commits at lists.llvm.org> wrote:
>
> Author: ruiu
> Date: Sun Oct  1 18:21:07 2017
> New Revision: 314644
>
> URL: http://llvm.org/viewvc/llvm-project?rev=314644&view=rev
> Log:
> [ICF] Include section contents in section hash values.
>
> Computing section content hashes early seems like a win in terms of
> performance. It increases a chance that two different sections will get
> different class IDs from the beginning.
>
> Without threads, this patch improves Chromium link time by about 0.3
> seconds. With threads, by 0.1 seconds. That's less than 1% time saving
> but not bad for a small patch.
>
> Modified:
>     lld/trunk/COFF/ICF.cpp
>     lld/trunk/ELF/ICF.cpp
>
> Modified: lld/trunk/COFF/ICF.cpp
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/ICF.cpp?
> rev=314644&r1=314643&r2=314644&view=diff
> ============================================================
> ==================
> --- lld/trunk/COFF/ICF.cpp (original)
> +++ lld/trunk/COFF/ICF.cpp Sun Oct  1 18:21:07 2017
> @@ -61,9 +61,9 @@ private:
>
>  // Returns a hash value for S.
>  uint32_t ICF::getHash(SectionChunk *C) {
> -  return hash_combine(C->getPermissions(), hash_value(C->SectionName),
> -                      C->NumRelocs, C->Alignment,
> -                      uint32_t(C->Header->SizeOfRawData), C->Checksum);
> +  return hash_combine(C->getPermissions(), C->SectionName, C->NumRelocs,
> +                      C->Alignment, uint32_t(C->Header->SizeOfRawData),
> +                      C->Checksum, C->getContents());
>  }
>
>  // Returns true if section S is subject of ICF.
> @@ -210,9 +210,10 @@ void ICF::run(const std::vector<Chunk *>
>    }
>
>    // Initially, we use hash values to partition sections.
> -  for (SectionChunk *SC : Chunks)
> +  for_each(parallel::par, Chunks.begin(), Chunks.end(), [&](SectionChunk
> *SC) {
>      // Set MSB to 1 to avoid collisions with non-hash classs.
>      SC->Class[0] = getHash(SC) | (1 << 31);
> +  });
>
>    // From now on, sections in Chunks are ordered so that sections in
>    // the same group are consecutive in the vector.
>
> Modified: lld/trunk/ELF/ICF.cpp
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/ICF.cpp?re
> v=314644&r1=314643&r2=314644&view=diff
> ============================================================
> ==================
> --- lld/trunk/ELF/ICF.cpp (original)
> +++ lld/trunk/ELF/ICF.cpp Sun Oct  1 18:21:07 2017
> @@ -155,7 +155,7 @@ private:
>  // Returns a hash value for S. Note that the information about
>  // relocation targets is not included in the hash value.
>  template <class ELFT> static uint32_t getHash(InputSection *S) {
> -  return hash_combine(S->Flags, S->getSize(), S->NumRelocations);
> +  return hash_combine(S->Flags, S->getSize(), S->NumRelocations, S->Data);
>  }
>
>  // Returns true if section S is subject of ICF.
> @@ -394,9 +394,10 @@ template <class ELFT> void ICF<ELFT>::ru
>          Sections.push_back(S);
>
>    // Initially, we use hash values to partition sections.
> -  for (InputSection *S : Sections)
> +  parallelForEach(Sections, [&](InputSection *S) {
>      // Set MSB to 1 to avoid collisions with non-hash IDs.
>      S->Class[0] = getHash<ELFT>(S) | (1 << 31);
> +  });
>
>    // From now on, sections in Sections vector are ordered so that sections
>    // in the same equivalence class are consecutive in the vector.
>
>
> _______________________________________________
> 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/20171002/487858d4/attachment.html>