[memprof] Improve the way we express Frames in YAML #119629
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This patch does two things: - During deserialization, we accept a function name for Frame as an alternative to the usual GUID expressed as a hexadecimal number. - During serialization, we print a GUID of Frame as a 16-digit hexadecimal number prefixed with 0x in the usual way. (Without this patch, we print a decimal number, which is not customary.) The patch uses a machinery called "normalization" in YAML I/O, which lets us serialize and deserialize into an alternative data structure. For our use case, we have an alternative Frame data structure, which is identical to "struct Frame" except that Function is of type GUIDHex64 instead of GlobalValue::GUID. This alternative type supports the two bullet points above without modifying "struct Frame" at all.
Member
|
@llvm/pr-subscribers-pgo Author: Kazu Hirata (kazutakahirata) ChangesThis patch does two things:
The patch uses a machinery called "normalization" in YAML I/O, which Full diff: https://github.com/llvm/llvm-project/pull/119629.diff 4 Files Affected:
diff --git a/llvm/include/llvm/ProfileData/MemProfYAML.h b/llvm/include/llvm/ProfileData/MemProfYAML.h
index 4568385fc6f71f..fa1b7dd4738435 100644
--- a/llvm/include/llvm/ProfileData/MemProfYAML.h
+++ b/llvm/include/llvm/ProfileData/MemProfYAML.h
@@ -52,11 +52,36 @@ template <> struct ScalarTraits<memprof::GUIDHex64> {
};
template <> struct MappingTraits<memprof::Frame> {
+ // Essentially the same as memprof::Frame except that Function is of type
+ // memprof::GUIDHex64 instead of GlobalValue::GUID. This class helps in two
+ // ways. During serialization, we print Function as a 16-digit hexadecimal
+ // number. During deserialization, we accept a function name as an
+ // alternative to the usual GUID expressed as a hexadecimal number.
+ class FrameWithHex64 {
+ public:
+ FrameWithHex64(IO &) {}
+ FrameWithHex64(IO &, const memprof::Frame &F)
+ : Function(F.Function), LineOffset(F.LineOffset), Column(F.Column),
+ IsInlineFrame(F.IsInlineFrame) {}
+ memprof::Frame denormalize(IO &) {
+ return memprof::Frame(Function, LineOffset, Column, IsInlineFrame);
+ }
+
+ memprof::GUIDHex64 Function = 0;
+ static_assert(std::is_same_v<decltype(Function.value),
+ decltype(memprof::Frame::Function)>);
+ decltype(memprof::Frame::LineOffset) LineOffset = 0;
+ decltype(memprof::Frame::Column) Column = 0;
+ decltype(memprof::Frame::IsInlineFrame) IsInlineFrame = false;
+ };
+
static void mapping(IO &Io, memprof::Frame &F) {
- Io.mapRequired("Function", F.Function);
- Io.mapRequired("LineOffset", F.LineOffset);
- Io.mapRequired("Column", F.Column);
- Io.mapRequired("IsInlineFrame", F.IsInlineFrame);
+ MappingNormalization<FrameWithHex64, memprof::Frame> Keys(Io, F);
+
+ Io.mapRequired("Function", Keys->Function);
+ Io.mapRequired("LineOffset", Keys->LineOffset);
+ Io.mapRequired("Column", Keys->Column);
+ Io.mapRequired("IsInlineFrame", Keys->IsInlineFrame);
// Assert that the definition of Frame matches what we expect. The
// structured bindings below detect changes to the number of fields.
diff --git a/llvm/test/tools/llvm-profdata/memprof-yaml.test b/llvm/test/tools/llvm-profdata/memprof-yaml.test
index 0c040ab3a1f9d9..c6ec7935ec64c8 100644
--- a/llvm/test/tools/llvm-profdata/memprof-yaml.test
+++ b/llvm/test/tools/llvm-profdata/memprof-yaml.test
@@ -11,24 +11,24 @@ HeapProfileRecords:
- GUID: 0xdeadbeef12345678
AllocSites:
- Callstack:
- - { Function: 100, LineOffset: 11, Column: 10, IsInlineFrame: true }
- - { Function: 200, LineOffset: 22, Column: 20, IsInlineFrame: false }
+ - { Function: 0x1111111111111111, LineOffset: 11, Column: 10, IsInlineFrame: true }
+ - { Function: 0x2222222222222222, LineOffset: 22, Column: 20, IsInlineFrame: false }
MemInfoBlock:
AllocCount: 111
TotalSize: 222
TotalLifetime: 333
TotalLifetimeAccessDensity: 444
- Callstack:
- - { Function: 300, LineOffset: 33, Column: 30, IsInlineFrame: false }
- - { Function: 400, LineOffset: 44, Column: 40, IsInlineFrame: true }
+ - { Function: 0x3333333333333333, LineOffset: 33, Column: 30, IsInlineFrame: false }
+ - { Function: 0x4444444444444444, LineOffset: 44, Column: 40, IsInlineFrame: true }
MemInfoBlock:
AllocCount: 555
TotalSize: 666
TotalLifetime: 777
TotalLifetimeAccessDensity: 888
CallSites:
- - - { Function: 500, LineOffset: 55, Column: 50, IsInlineFrame: true }
- - { Function: 600, LineOffset: 66, Column: 60, IsInlineFrame: false }
- - - { Function: 700, LineOffset: 77, Column: 70, IsInlineFrame: true }
- - { Function: 800, LineOffset: 88, Column: 80, IsInlineFrame: false }
+ - - { Function: 0x5555555555555555, LineOffset: 55, Column: 50, IsInlineFrame: true }
+ - { Function: 0x6666666666666666, LineOffset: 66, Column: 60, IsInlineFrame: false }
+ - - { Function: 0x7777777777777777, LineOffset: 77, Column: 70, IsInlineFrame: true }
+ - { Function: 0x8888888888888888, LineOffset: 88, Column: 80, IsInlineFrame: false }
...
diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp
index 6b9e2349899a44..ffc481f071857a 100644
--- a/llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -3307,7 +3307,9 @@ static int showMemProfProfile(ShowFormat SFormat, raw_fd_ostream &OS) {
auto Reader = std::move(ReaderOrErr.get());
memprof::AllMemProfData Data = Reader->getAllMemProfData();
- yaml::Output Yout(OS);
+ // Construct yaml::Output with the maximum column width of 80 so that each
+ // Frame fits in one line.
+ yaml::Output Yout(OS, nullptr, 80);
Yout << Data;
return 0;
diff --git a/llvm/unittests/ProfileData/MemProfTest.cpp b/llvm/unittests/ProfileData/MemProfTest.cpp
index 456b093362b50f..7dac0eb7ca87f2 100644
--- a/llvm/unittests/ProfileData/MemProfTest.cpp
+++ b/llvm/unittests/ProfileData/MemProfTest.cpp
@@ -804,11 +804,11 @@ template <typename T> std::string serializeInYAML(T &Val) {
}
TEST(MemProf, YAMLWriterFrame) {
- Frame F(11, 22, 33, true);
+ Frame F(0x0123456789abcdefULL, 22, 33, true);
std::string Out = serializeInYAML(F);
EXPECT_EQ(Out, R"YAML(---
-{ Function: 11, LineOffset: 22, Column: 33, IsInlineFrame: true }
+{ Function: 0x0123456789abcdef, LineOffset: 22, Column: 33, IsInlineFrame: true }
...
)YAML");
}
|
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/10108 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch does two things:
During deserialization, we accept a function name for Frame as an
alternative to the usual GUID expressed as a hexadecimal number.
During serialization, we print a GUID of Frame as a 16-digit
hexadecimal number prefixed with 0x in the usual way. (Without this
patch, we print a decimal number, which is not customary.)
The patch uses a machinery called "normalization" in YAML I/O, which
lets us serialize and deserialize into an alternative data structure.
For our use case, we have an alternative Frame data structure, which
is identical to "struct Frame" except that Function is of type
GUIDHex64 instead of GlobalValue::GUID. This alternative type
supports the two bullet points above without modifying "struct Frame"
at all.