From c9a8a0e8a9b267fae0352af352e9c735fef019f7 Mon Sep 17 00:00:00 2001 From: Douglas Yung Date: Fri, 23 Jun 2023 17:58:22 -0700 Subject: [PATCH] Revert "[llvm-profdata] Refactoring Sample Profile Reader to increase FDO build speed using MD5 as key to Sample Profile map" This reverts commit 31af18bccea95fe1ae8aa2c51cf7c8e92a1c208e. This change is causing build failures on many Windows build bots: https://lab.llvm.org/buildbot/#/builders/216/builds/22833 https://lab.llvm.org/buildbot/#/builders/123/builds/19602 https://lab.llvm.org/buildbot/#/builders/172/builds/28315 https://lab.llvm.org/buildbot/#/builders/119/builds/13870 https://lab.llvm.org/buildbot/#/builders/233/builds/794 https://lab.llvm.org/buildbot/#/builders/235/builds/387 https://lab.llvm.org/buildbot/#/builders/13/builds/36921 https://lab.llvm.org/buildbot/#/builders/127/builds/50510 --- llvm/include/llvm/ProfileData/SampleProf.h | 211 ++------------------- llvm/include/llvm/ProfileData/SampleProfReader.h | 52 +++-- llvm/lib/ProfileData/ProfileSummaryBuilder.cpp | 5 +- llvm/lib/ProfileData/SampleProf.cpp | 74 ++++++-- llvm/lib/ProfileData/SampleProfReader.cpp | 121 ++++-------- llvm/lib/ProfileData/SampleProfWriter.cpp | 4 +- llvm/lib/Transforms/IPO/SampleContextTracker.cpp | 4 +- .../Inputs/sample-nametable-after-samples.profdata | Bin 96 -> 96 bytes .../test/tools/llvm-profdata/sample-nametable.test | 2 +- llvm/tools/llvm-profdata/llvm-profdata.cpp | 5 +- llvm/tools/llvm-profgen/ProfileGenerator.cpp | 19 +- llvm/unittests/tools/llvm-profdata/CMakeLists.txt | 1 - .../tools/llvm-profdata/MD5CollisionTest.cpp | 166 ---------------- .../tools/llvm-profdata/OutputSizeLimitTest.cpp | 2 +- 14 files changed, 153 insertions(+), 513 deletions(-) delete mode 100644 llvm/unittests/tools/llvm-profdata/MD5CollisionTest.cpp diff --git a/llvm/include/llvm/ProfileData/SampleProf.h b/llvm/include/llvm/ProfileData/SampleProf.h index 26fd386..f92b993 100644 --- a/llvm/include/llvm/ProfileData/SampleProf.h +++ b/llvm/include/llvm/ProfileData/SampleProf.h @@ -318,14 +318,6 @@ struct LineLocationHash { raw_ostream &operator<<(raw_ostream &OS, const LineLocation &Loc); -static inline hash_code hashFuncName(StringRef F) { - // If function name is already MD5 string, do not hash again. - uint64_t Hash; - if (F.getAsInteger(10, Hash)) - Hash = MD5Hash(F); - return Hash; -} - /// Representation of a single sample record. /// /// A sample record is represented by a positive integer value, which @@ -638,13 +630,9 @@ public: return getContextString(FullContext, false); } - hash_code getHashCode() const { - if (hasContext()) - return hash_value(getContextFrames()); - - // For non-context function name, use its MD5 as hash value, so that it is - // consistent with the profile map's key. - return hashFuncName(getName()); + uint64_t getHashCode() const { + return hasContext() ? hash_value(getContextFrames()) + : hash_value(getName()); } /// Set the name of the function and clear the current context. @@ -722,12 +710,9 @@ private: uint32_t Attributes; }; -static inline hash_code hash_value(const SampleContext &Context) { - return Context.getHashCode(); -} - -inline raw_ostream &operator<<(raw_ostream &OS, const SampleContext &Context) { - return OS << Context.toString(); +static inline hash_code hash_value(const SampleContext &arg) { + return arg.hasContext() ? hash_value(arg.getContextFrames()) + : hash_value(arg.getName()); } class FunctionSamples; @@ -1217,9 +1202,6 @@ public: return !(*this == Other); } - template - const T &getKey() const; - private: /// CFG hash value for the function. uint64_t FunctionHash = 0; @@ -1283,173 +1265,12 @@ private: const LocToLocMap *IRToProfileLocationMap = nullptr; }; -template <> -inline const SampleContext &FunctionSamples::getKey() const { - return getContext(); -} - raw_ostream &operator<<(raw_ostream &OS, const FunctionSamples &FS); -/// This class is a wrapper to associative container MapT using -/// the hash value of the original key as the new key. This greatly improves the -/// performance of insert and query operations especially when hash values of -/// keys are available a priori, and reduces memory usage if KeyT has a large -/// size. -/// When performing any action, if an existing entry with a given key is found, -/// and the interface "KeyT ValueT::getKey() const" to retrieve a value's -/// original key exists, this class checks if the given key actually matches -/// the existing entry's original key. If they do not match, this class behaves -/// as if the entry did not exist (for insertion, this means the new value will -/// replace the existing entry's value, as if it is newly inserted). If -/// ValueT::getKey() is not available, all keys with the same hash value -/// are considered equivalent (i.e. hash collision is silently ignored). Given -/// such feature this class should only be used where it does not affect -/// compilation correctness, for example, when loading a sample profile. -/// Assuming the hashing algorithm is uniform, the probability of hash collision -/// with 1,000,000 entries is -/// (2^64)!/((2^64-1000000)!*(2^64)^1000000) ~= 3*10^-8. -template