Skip to content

[memprof] Add accessors to Frame::SymbolName #94085

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

kazutakahirata
Copy link
Contributor

This patch adds accessors to Frame::SymbolName so that we can change
the underlying type of SymbolName without affecting downstream users
once they switch to the new accessors.

Note that SymbolName is only used for debugging. Changing the type of
SymbolName from std::optionalstd::string to
std::unique_ptrstd::string cuts down sizeof(Frame) by half -- from
64 bytes to 32 bytes. (std::optional sets aside the storage in
case T is instantiated.)

During deserialization, the memory usage is dominated by Frames.
Shrinking the type cuts down the memory usage and deserialization time
nearly by half.

This patch adds accessors to Frame::SymbolName so that we can change
the underlying type of SymbolName without affecting downstream users
once they switch to the new accessors.

Note that SymbolName is only used for debugging.  Changing the type of
SymbolName from std::optional<std::string> to
std::unique_ptr<std::string> cuts down sizeof(Frame) by half -- from
64 bytes to 32 bytes.  (std::optional<T> sets aside the storage in
case T is instantiated.)

During deserialization, the memory usage is dominated by Frames.
Shrinking the type cuts down the memory usage and deserialization time
nearly by half.
@llvmbot llvmbot added the PGO Profile Guided Optimizations label Jun 1, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 1, 2024

@llvm/pr-subscribers-pgo

Author: Kazu Hirata (kazutakahirata)

Changes

This patch adds accessors to Frame::SymbolName so that we can change
the underlying type of SymbolName without affecting downstream users
once they switch to the new accessors.

Note that SymbolName is only used for debugging. Changing the type of
SymbolName from std::optional<std::string> to
std::unique_ptr<std::string> cuts down sizeof(Frame) by half -- from
64 bytes to 32 bytes. (std::optional<T> sets aside the storage in
case T is instantiated.)

During deserialization, the memory usage is dominated by Frames.
Shrinking the type cuts down the memory usage and deserialization time
nearly by half.


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

1 Files Affected:

  • (modified) llvm/include/llvm/ProfileData/MemProf.h (+12-1)
diff --git a/llvm/include/llvm/ProfileData/MemProf.h b/llvm/include/llvm/ProfileData/MemProf.h
index e8683e8ac2b95..7b7495035fe11 100644
--- a/llvm/include/llvm/ProfileData/MemProf.h
+++ b/llvm/include/llvm/ProfileData/MemProf.h
@@ -237,6 +237,17 @@ struct Frame {
 
   bool operator!=(const Frame &Other) const { return !operator==(Other); }
 
+  bool hasSymbolName() const { return SymbolName.has_value(); }
+
+  StringRef getSymbolName() const {
+    assert(SymbolName.has_value());
+    return *SymbolName;
+  }
+
+  std::string getSymbolNameOr(StringRef Alt) const {
+    return std::string(hasSymbolName() ? getSymbolName() : Alt);
+  }
+
   // Write the contents of the frame to the ostream \p OS.
   void serialize(raw_ostream &OS) const {
     using namespace support;
@@ -279,7 +290,7 @@ struct Frame {
   void printYAML(raw_ostream &OS) const {
     OS << "      -\n"
        << "        Function: " << Function << "\n"
-       << "        SymbolName: " << SymbolName.value_or("<None>") << "\n"
+       << "        SymbolName: " << getSymbolNameOr("<None>") << "\n"
        << "        LineOffset: " << LineOffset << "\n"
        << "        Column: " << Column << "\n"
        << "        Inline: " << IsInlineFrame << "\n";

@kazutakahirata kazutakahirata merged commit f367eaa into llvm:main Jun 3, 2024
9 checks passed
@kazutakahirata kazutakahirata deleted the memprof_SymbolName_accessors branch June 3, 2024 05:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PGO Profile Guided Optimizations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants