From d16a61d2c84b73144f232dcd75b6c5e03f9f898c Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Thu, 29 Jun 2017 02:51:58 +0000 Subject: [PATCH] llvm-profdata: Indirect infrequently used fields to reduce memory usage Examining a large profile example, it seems relatively few records have non-empty IndirectCall and MemOP data, so indirecting these through a unique_ptr (non-null only when they are non-empty) Reduces memory usage on this particular example from 14GB to 10GB according to valgrind's massif. I suspect it'd still be worth moving InstrProfWriter to its own data structure that had Counts and the indirected IndirectCall+MemOP, and did not include the Name, Hash, or Error fields. This would reduce the size of this dominant data structure by half of this new, lower amount. (Name(2), Hash(1), Error(1) ~= Counts(vector, 3), ValueProfData (unique_ptr, 1)) -> From code review feedback, might actually refactor InstrProfRecord itself to have a sub-struct with all the counts, and use that from InstrProfWriter, rather than InstrProfWriter owning its own data structure for this. Reviewers: davidxl Differential Revision: https://reviews.llvm.org/D34694 llvm-svn: 306631 --- llvm/include/llvm/ProfileData/InstrProf.h | 81 +++++++++++++++++------ llvm/lib/ProfileData/InstrProf.cpp | 15 ++--- 2 files changed, 66 insertions(+), 30 deletions(-) diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h index 573ea90cfd00..a6b2850ccd22 100644 --- a/llvm/include/llvm/ProfileData/InstrProf.h +++ b/llvm/include/llvm/ProfileData/InstrProf.h @@ -598,6 +598,28 @@ struct InstrProfRecord { InstrProfRecord() = default; InstrProfRecord(StringRef Name, uint64_t Hash, std::vector Counts) : Name(Name), Hash(Hash), Counts(std::move(Counts)) {} + InstrProfRecord(InstrProfRecord &&) = default; + InstrProfRecord(const InstrProfRecord &RHS) + : Name(RHS.Name), Hash(RHS.Hash), Counts(RHS.Counts), SIPE(RHS.SIPE), + ValueData(RHS.ValueData + ? llvm::make_unique(*RHS.ValueData) + : nullptr) {} + InstrProfRecord &operator=(InstrProfRecord &&) = default; + InstrProfRecord &operator=(const InstrProfRecord &RHS) { + Name = RHS.Name; + Hash = RHS.Hash; + Counts = RHS.Counts; + SIPE = RHS.SIPE; + if (!RHS.ValueData) { + ValueData = nullptr; + return *this; + } + if (!ValueData) + ValueData = llvm::make_unique(*RHS.ValueData); + else + *ValueData = *RHS.ValueData; + return *this; + } using ValueMapType = std::vector>; @@ -647,12 +669,9 @@ struct InstrProfRecord { /// Sort value profile data (per site) by count. void sortValueData() { - for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; ++Kind) { - std::vector &SiteRecords = - getValueSitesForKind(Kind); - for (auto &SR : SiteRecords) + for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; ++Kind) + for (auto &SR : getValueSitesForKind(Kind)) SR.sortByCount(); - } } /// Clear value data entries and edge counters. @@ -662,36 +681,54 @@ struct InstrProfRecord { } /// Clear value data entries - void clearValueData() { - for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; ++Kind) - getValueSitesForKind(Kind).clear(); - } + void clearValueData() { ValueData = nullptr; } /// Get the error contained within the record's soft error counter. Error takeError() { return SIPE.takeError(); } private: - std::vector IndirectCallSites; - std::vector MemOPSizes; + struct ValueProfData { + std::vector IndirectCallSites; + std::vector MemOPSizes; + }; + std::unique_ptr ValueData; - const std::vector & + MutableArrayRef + getValueSitesForKind(uint32_t ValueKind) { + // Cast to /add/ const (should be an implicit_cast, ideally, if that's ever + // implemented in LLVM) to call the const overload of this function, then + // cast away the constness from the result. + auto AR = const_cast(this)->getValueSitesForKind( + ValueKind); + return makeMutableArrayRef( + const_cast(AR.data()), AR.size()); + } + ArrayRef getValueSitesForKind(uint32_t ValueKind) const { + if (!ValueData) + return None; switch (ValueKind) { case IPVK_IndirectCallTarget: - return IndirectCallSites; + return ValueData->IndirectCallSites; case IPVK_MemOPSize: - return MemOPSizes; + return ValueData->MemOPSizes; default: llvm_unreachable("Unknown value kind!"); } - return IndirectCallSites; } std::vector & - getValueSitesForKind(uint32_t ValueKind) { - return const_cast &>( - const_cast(this) - ->getValueSitesForKind(ValueKind)); + getOrCreateValueSitesForKind(uint32_t ValueKind) { + if (!ValueData) + ValueData = llvm::make_unique(); + switch (ValueKind) { + case IPVK_IndirectCallTarget: + return ValueData->IndirectCallSites; + case IPVK_MemOPSize: + return ValueData->MemOPSizes; + default: + llvm_unreachable("Unknown value kind!"); + } } // Map indirect call target name hash to name string. @@ -765,9 +802,9 @@ uint64_t InstrProfRecord::getValueForSite(InstrProfValueData Dest[], } void InstrProfRecord::reserveSites(uint32_t ValueKind, uint32_t NumValueSites) { - std::vector &ValueSites = - getValueSitesForKind(ValueKind); - ValueSites.reserve(NumValueSites); + if (!NumValueSites) + return; + getOrCreateValueSitesForKind(ValueKind).reserve(NumValueSites); } inline support::endianness getHostEndianness() { diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp index 1dc596fa7ed6..a1d18724fcd5 100644 --- a/llvm/lib/ProfileData/InstrProf.cpp +++ b/llvm/lib/ProfileData/InstrProf.cpp @@ -504,9 +504,11 @@ void InstrProfRecord::mergeValueProfData(uint32_t ValueKind, SIPE.addError(instrprof_error::value_site_count_mismatch); return; } + if (!ThisNumValueSites) + return; std::vector &ThisSiteRecords = - getValueSitesForKind(ValueKind); - std::vector &OtherSiteRecords = + getOrCreateValueSitesForKind(ValueKind); + MutableArrayRef OtherSiteRecords = Src.getValueSitesForKind(ValueKind); for (uint32_t I = 0; I < ThisNumValueSites; I++) ThisSiteRecords[I].merge(SIPE, OtherSiteRecords[I], Weight); @@ -533,11 +535,8 @@ void InstrProfRecord::merge(InstrProfRecord &Other, uint64_t Weight) { } void InstrProfRecord::scaleValueProfData(uint32_t ValueKind, uint64_t Weight) { - uint32_t ThisNumValueSites = getNumValueSites(ValueKind); - std::vector &ThisSiteRecords = - getValueSitesForKind(ValueKind); - for (uint32_t I = 0; I < ThisNumValueSites; I++) - ThisSiteRecords[I].scale(SIPE, Weight); + for (auto &R : getValueSitesForKind(ValueKind)) + R.scale(SIPE, Weight); } void InstrProfRecord::scale(uint64_t Weight) { @@ -583,7 +582,7 @@ void InstrProfRecord::addValueData(uint32_t ValueKind, uint32_t Site, VData[I].Value = remapValue(VData[I].Value, ValueKind, ValueMap); } std::vector &ValueSites = - getValueSitesForKind(ValueKind); + getOrCreateValueSitesForKind(ValueKind); if (N == 0) ValueSites.emplace_back(); else