Skip to content

[BOLT][NFC] Make YamlProfileToFunction a DenseMap #108712

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

aaupov
Copy link
Contributor

@aaupov aaupov commented Sep 14, 2024

YAML function profiles have sparse function IDs, assigned from
sequential function IDs from profiled binary. For example, for one large
binary, YAML profile has 15K functions, but the highest ID is ~600K,
close to number of functions in the profiled binary.

In matchProfileToFunction, YamlProfileToFunction vector was resized
to match function ID, which entails a 40X overcommit. Change the type of
YamlProfileToFunction to DenseMap to reduce memory utilization.

#99891 makes use of it for profile lookup associated with a given binary
function.

Created using spr 1.3.4
@aaupov aaupov marked this pull request as ready for review September 16, 2024 04:28
@llvmbot llvmbot added the BOLT label Sep 16, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 16, 2024

@llvm/pr-subscribers-bolt

Author: Amir Ayupov (aaupov)

Changes

YAML function profiles have sparse function IDs, assigned from
sequential function IDs from profiled binary. For example, for one large
binary, YAML profile has 15K functions, but the highest ID is ~600K,
close to number of functions in the profiled binary.

In matchProfileToFunction, YamlProfileToFunction vector was resized
to match function ID, which entails a 40X overcommit.

Change the type of YamlProfileToFunction to DenseMap to curb memory
utilization.


Full diff: https://github.com/llvm/llvm-project/pull/108712.diff

2 Files Affected:

  • (modified) bolt/include/bolt/Profile/YAMLProfileReader.h (+1-3)
  • (modified) bolt/lib/Profile/YAMLProfileReader.cpp (+3-10)
diff --git a/bolt/include/bolt/Profile/YAMLProfileReader.h b/bolt/include/bolt/Profile/YAMLProfileReader.h
index bd5a86fd676a59..a6f0fd6f3251f0 100644
--- a/bolt/include/bolt/Profile/YAMLProfileReader.h
+++ b/bolt/include/bolt/Profile/YAMLProfileReader.h
@@ -105,7 +105,7 @@ class YAMLProfileReader : public ProfileReaderBase {
   yaml::bolt::BinaryProfile YamlBP;
 
   /// Map a function ID from a YAML profile to a BinaryFunction object.
-  std::vector<BinaryFunction *> YamlProfileToFunction;
+  DenseMap<uint32_t, BinaryFunction *> YamlProfileToFunction;
 
   using FunctionSet = std::unordered_set<const BinaryFunction *>;
   /// To keep track of functions that have a matched profile before the profile
@@ -162,8 +162,6 @@ class YAMLProfileReader : public ProfileReaderBase {
   /// Update matched YAML -> BinaryFunction pair.
   void matchProfileToFunction(yaml::bolt::BinaryFunctionProfile &YamlBF,
                               BinaryFunction &BF) {
-    if (YamlBF.Id >= YamlProfileToFunction.size())
-      YamlProfileToFunction.resize(YamlBF.Id + 1);
     YamlProfileToFunction[YamlBF.Id] = &BF;
     YamlBF.Used = true;
 
diff --git a/bolt/lib/Profile/YAMLProfileReader.cpp b/bolt/lib/Profile/YAMLProfileReader.cpp
index 67ed32017667d6..3bd09508fb76e1 100644
--- a/bolt/lib/Profile/YAMLProfileReader.cpp
+++ b/bolt/lib/Profile/YAMLProfileReader.cpp
@@ -238,9 +238,7 @@ bool YAMLProfileReader::parseFunctionProfile(
     BB.setExecutionCount(YamlBB.ExecCount);
 
     for (const yaml::bolt::CallSiteInfo &YamlCSI : YamlBB.CallSites) {
-      BinaryFunction *Callee = YamlCSI.DestId < YamlProfileToFunction.size()
-                                   ? YamlProfileToFunction[YamlCSI.DestId]
-                                   : nullptr;
+      BinaryFunction *Callee = YamlProfileToFunction.lookup(YamlCSI.DestId);
       bool IsFunction = Callee ? true : false;
       MCSymbol *CalleeSymbol = nullptr;
       if (IsFunction)
@@ -707,7 +705,7 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
       break;
     }
   }
-  YamlProfileToFunction.resize(YamlBP.Functions.size() + 1);
+  YamlProfileToFunction.reserve(YamlBP.Functions.size());
 
   // Computes hash for binary functions.
   if (opts::MatchProfileWithFunctionHash) {
@@ -760,12 +758,7 @@ Error YAMLProfileReader::readProfile(BinaryContext &BC) {
   NormalizeByCalls = usesEvent("branches");
   uint64_t NumUnused = 0;
   for (yaml::bolt::BinaryFunctionProfile &YamlBF : YamlBP.Functions) {
-    if (YamlBF.Id >= YamlProfileToFunction.size()) {
-      // Such profile was ignored.
-      ++NumUnused;
-      continue;
-    }
-    if (BinaryFunction *BF = YamlProfileToFunction[YamlBF.Id])
+    if (BinaryFunction *BF = YamlProfileToFunction.lookup(YamlBF.Id))
       parseFunctionProfile(*BF, YamlBF);
     else
       ++NumUnused;

@@ -105,7 +105,7 @@ class YAMLProfileReader : public ProfileReaderBase {
yaml::bolt::BinaryProfile YamlBP;

/// Map a function ID from a YAML profile to a BinaryFunction object.
std::vector<BinaryFunction *> YamlProfileToFunction;
DenseMap<uint32_t, BinaryFunction *> YamlProfileToFunction;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious if using uint32_t leads to more compact allocation compared to uint64_t?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it does: DenseMap stores objects in buckets where bucket is a KV pair where the second element (pointer) would force the alignment to be 8b.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the YamlBF.Id is uint32, so I decided to match it:

struct BinaryFunctionProfile {
std::string Name;
uint32_t NumBasicBlocks{0};
uint32_t Id{0};

@aaupov aaupov merged commit d936924 into main Nov 8, 2024
11 checks passed
@aaupov aaupov deleted the users/aaupov/spr/boltnfc-make-yamlprofiletofunction-a-densemap branch November 8, 2024 23:24
Groverkss pushed a commit to iree-org/llvm-project that referenced this pull request Nov 15, 2024
YAML function profiles have sparse function IDs, assigned from
sequential function IDs from profiled binary. For example, for one large
binary, YAML profile has 15K functions, but the highest ID is ~600K,
close to number of functions in the profiled binary.

In `matchProfileToFunction`, `YamlProfileToFunction` vector was resized
to match function ID, which entails a 40X overcommit. Change the type of
`YamlProfileToFunction` to DenseMap to reduce memory utilization.

llvm#99891 makes use of it for profile lookup associated with a given binary
function.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants