-
Notifications
You must be signed in to change notification settings - Fork 14.7k
[MemProf] Write out raw profile bytes in little endian. #150375
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
[MemProf] Write out raw profile bytes in little endian. #150375
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
} | ||
#if defined(__BYTE_ORDER__) && __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ | ||
for (size_t i = 0; i < sizeof(T) / 2; ++i) { | ||
std::swap(buffer[i], buffer[sizeof(T) - 1 - i]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
buffer should be Buffer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alternatively, copy in from Src above in the current direction if little endian, and in reverse order if big endian (rather than copy and swap in the BE case)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
a3475c3
to
6ab7208
Compare
d2a1048
to
dbc7ec3
Compare
6ab7208
to
0eca4b1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ptal, thanks.
} | ||
#if defined(__BYTE_ORDER__) && __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ | ||
for (size_t i = 0; i < sizeof(T) / 2; ++i) { | ||
std::swap(buffer[i], buffer[sizeof(T) - 1 - i]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@llvm/pr-subscribers-pgo Author: Snehasish Kumar (snehasish) ChangesInstead of writing out in native endian, write out the raw profile bytes Full diff: https://github.com/llvm/llvm-project/pull/150375.diff 2 Files Affected:
diff --git a/compiler-rt/lib/memprof/memprof_rawprofile.cpp b/compiler-rt/lib/memprof/memprof_rawprofile.cpp
index f909d78f5f36a..bf04afa679c9c 100644
--- a/compiler-rt/lib/memprof/memprof_rawprofile.cpp
+++ b/compiler-rt/lib/memprof/memprof_rawprofile.cpp
@@ -7,10 +7,7 @@
#include "sanitizer_common/sanitizer_allocator_internal.h"
#include "sanitizer_common/sanitizer_array_ref.h"
#include "sanitizer_common/sanitizer_common.h"
-#include "sanitizer_common/sanitizer_linux.h"
-#include "sanitizer_common/sanitizer_procmaps.h"
#include "sanitizer_common/sanitizer_stackdepot.h"
-#include "sanitizer_common/sanitizer_stackdepotbase.h"
#include "sanitizer_common/sanitizer_stacktrace.h"
#include "sanitizer_common/sanitizer_vector.h"
@@ -23,7 +20,16 @@ using ::llvm::memprof::encodeHistogramCount;
namespace {
template <class T> char *WriteBytes(const T &Pod, char *Buffer) {
- *(T *)Buffer = Pod;
+ static_assert(is_trivially_copyable<T>::value, "T must be POD");
+ const uint8_t *Src = reinterpret_cast<const uint8_t *>(&Pod);
+
+ for (size_t I = 0; I < sizeof(T); ++I)
+#if defined(__BYTE_ORDER__) && __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+ // Reverse byte order since reader is little-endian.
+ Buffer[I] = Src[sizeof(T) - 1 - I];
+#else
+ Buffer[I] = Src[I];
+#endif
return Buffer + sizeof(T);
}
@@ -33,7 +39,6 @@ void RecordStackId(const uptr Key, UNUSED LockedMemInfoBlock *const &MIB,
auto *StackIds = reinterpret_cast<Vector<u64> *>(Arg);
StackIds->PushBack(Key);
}
-} // namespace
u64 SegmentSizeBytes(ArrayRef<LoadedModule> Modules) {
u64 NumSegmentsToRecord = 0;
@@ -184,6 +189,7 @@ void SerializeMIBInfoToBuffer(MIBMapTy &MIBMap, const Vector<u64> &StackIds,
CHECK(ExpectedNumBytes >= static_cast<u64>(Ptr - Buffer) &&
"Expected num bytes != actual bytes written");
}
+} // namespace
// Format
// ---------- Header
@@ -288,5 +294,4 @@ u64 SerializeToRawProfile(MIBMapTy &MIBMap, ArrayRef<LoadedModule> Modules,
return TotalSizeBytes;
}
-
} // namespace __memprof
diff --git a/llvm/lib/ProfileData/MemProfReader.cpp b/llvm/lib/ProfileData/MemProfReader.cpp
index 9db699712d6f3..3fc0dbfd8e69d 100644
--- a/llvm/lib/ProfileData/MemProfReader.cpp
+++ b/llvm/lib/ProfileData/MemProfReader.cpp
@@ -146,8 +146,39 @@ readMemInfoBlocksCommon(const char *Ptr, bool IsHistogramEncoded = false) {
const uint64_t Id =
endian::readNext<uint64_t, llvm::endianness::little, unaligned>(Ptr);
- MemInfoBlock MIB = *reinterpret_cast<const MemInfoBlock *>(Ptr);
- Ptr += sizeof(MemInfoBlock);
+ MemInfoBlock MIB;
+#define READ_MIB_FIELD(FIELD) \
+ MIB.FIELD = endian::readNext<decltype(MIB.FIELD), llvm::endianness::little, \
+ unaligned>(Ptr)
+
+ READ_MIB_FIELD(AllocCount);
+ READ_MIB_FIELD(TotalAccessCount);
+ READ_MIB_FIELD(MinAccessCount);
+ READ_MIB_FIELD(MaxAccessCount);
+ READ_MIB_FIELD(TotalSize);
+ READ_MIB_FIELD(MinSize);
+ READ_MIB_FIELD(MaxSize);
+ READ_MIB_FIELD(AllocTimestamp);
+ READ_MIB_FIELD(DeallocTimestamp);
+ READ_MIB_FIELD(TotalLifetime);
+ READ_MIB_FIELD(MinLifetime);
+ READ_MIB_FIELD(MaxLifetime);
+ READ_MIB_FIELD(AllocCpuId);
+ READ_MIB_FIELD(DeallocCpuId);
+ READ_MIB_FIELD(NumMigratedCpu);
+ READ_MIB_FIELD(NumLifetimeOverlaps);
+ READ_MIB_FIELD(NumSameAllocCpu);
+ READ_MIB_FIELD(NumSameDeallocCpu);
+ READ_MIB_FIELD(DataTypeId);
+ READ_MIB_FIELD(TotalAccessDensity);
+ READ_MIB_FIELD(MinAccessDensity);
+ READ_MIB_FIELD(MaxAccessDensity);
+ READ_MIB_FIELD(TotalLifetimeAccessDensity);
+ READ_MIB_FIELD(MinLifetimeAccessDensity);
+ READ_MIB_FIELD(MaxLifetimeAccessDensity);
+ READ_MIB_FIELD(AccessHistogramSize);
+ READ_MIB_FIELD(AccessHistogram);
+#undef READ_MIB_FIELD
if (MIB.AccessHistogramSize > 0) {
// The in-memory representation uses uint64_t for histogram entries.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm but suggest updating the title and/or description to note this is for MemProf.
f176941
to
e815865
Compare
dac3d49
to
3129de2
Compare
e815865
to
75ea9cd
Compare
13343c6
to
2805780
Compare
Instead of writing out in native endian, write out the raw profile bytes in little endian. Also update the MIB data in little endian. Also clean up some lint and unused includes in rawprofile.cpp.
75ea9cd
to
a4b7a90
Compare
Instead of writing out in native endian, write out the raw profile bytes
in little endian. Also update the MIB data in little endian. Also clean
up some lint and unused includes in rawprofile.cpp.