[memprof] Move IndexedMemProfReader::deserialize to IndexedemProfData.cpp (NFC) #137089
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
….cpp (NFC) This patch moves IndexedMemProfReader::deserialize and its subroutines to IndexedemProfData.cpp, building on: commit 9a8f90d Author: Kazu Hirata <[email protected]> Date: Wed Apr 23 15:39:45 2025 -0700 The intent is as follows: - Reduce the size of InstrProfReader.cpp. - Move the subroutines to a separate file because they don't interact with anything else in InstrProfReader.cpp.
kazutakahirata
requested review from
snehasish,
teresajohnson and
mingmingl-llvm
|
@llvm/pr-subscribers-pgo Author: Kazu Hirata (kazutakahirata) ChangesThis patch moves IndexedMemProfReader::deserialize and its subroutines commit 9a8f90d The intent is as follows:
Full diff: https://github.com/llvm/llvm-project/pull/137089.diff 2 Files Affected:
diff --git a/llvm/lib/ProfileData/IndexedMemProfData.cpp b/llvm/lib/ProfileData/IndexedMemProfData.cpp
index fb4a891a2eb95..5e78ffdb86d67 100644
--- a/llvm/lib/ProfileData/IndexedMemProfData.cpp
+++ b/llvm/lib/ProfileData/IndexedMemProfData.cpp
@@ -11,6 +11,7 @@
//===----------------------------------------------------------------------===//
#include "llvm/ProfileData/InstrProf.h"
+#include "llvm/ProfileData/InstrProfReader.h"
#include "llvm/ProfileData/MemProf.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/OnDiskHashTable.h"
@@ -297,4 +298,127 @@ Error writeMemProf(ProfOStream &OS, memprof::IndexedMemProfData &MemProfData,
memprof::MaximumSupportedVersion));
}
+Error IndexedMemProfReader::deserializeV2(const unsigned char *Start,
+ const unsigned char *Ptr) {
+ // The value returned from RecordTableGenerator.Emit.
+ const uint64_t RecordTableOffset =
+ support::endian::readNext<uint64_t, llvm::endianness::little>(Ptr);
+ // The offset in the stream right before invoking
+ // FrameTableGenerator.Emit.
+ const uint64_t FramePayloadOffset =
+ support::endian::readNext<uint64_t, llvm::endianness::little>(Ptr);
+ // The value returned from FrameTableGenerator.Emit.
+ const uint64_t FrameTableOffset =
+ support::endian::readNext<uint64_t, llvm::endianness::little>(Ptr);
+
+ // The offset in the stream right before invoking
+ // CallStackTableGenerator.Emit.
+ uint64_t CallStackPayloadOffset = 0;
+ // The value returned from CallStackTableGenerator.Emit.
+ uint64_t CallStackTableOffset = 0;
+ if (Version >= memprof::Version2) {
+ CallStackPayloadOffset =
+ support::endian::readNext<uint64_t, llvm::endianness::little>(Ptr);
+ CallStackTableOffset =
+ support::endian::readNext<uint64_t, llvm::endianness::little>(Ptr);
+ }
+
+ // Read the schema.
+ auto SchemaOr = memprof::readMemProfSchema(Ptr);
+ if (!SchemaOr)
+ return SchemaOr.takeError();
+ Schema = SchemaOr.get();
+
+ // Now initialize the table reader with a pointer into data buffer.
+ MemProfRecordTable.reset(MemProfRecordHashTable::Create(
+ /*Buckets=*/Start + RecordTableOffset,
+ /*Payload=*/Ptr,
+ /*Base=*/Start, memprof::RecordLookupTrait(Version, Schema)));
+
+ // Initialize the frame table reader with the payload and bucket offsets.
+ MemProfFrameTable.reset(MemProfFrameHashTable::Create(
+ /*Buckets=*/Start + FrameTableOffset,
+ /*Payload=*/Start + FramePayloadOffset,
+ /*Base=*/Start));
+
+ if (Version >= memprof::Version2)
+ MemProfCallStackTable.reset(MemProfCallStackHashTable::Create(
+ /*Buckets=*/Start + CallStackTableOffset,
+ /*Payload=*/Start + CallStackPayloadOffset,
+ /*Base=*/Start));
+
+ return Error::success();
+}
+
+Error IndexedMemProfReader::deserializeV3(const unsigned char *Start,
+ const unsigned char *Ptr) {
+ // The offset in the stream right before invoking
+ // CallStackTableGenerator.Emit.
+ const uint64_t CallStackPayloadOffset =
+ support::endian::readNext<uint64_t, llvm::endianness::little>(Ptr);
+ // The offset in the stream right before invoking RecordTableGenerator.Emit.
+ const uint64_t RecordPayloadOffset =
+ support::endian::readNext<uint64_t, llvm::endianness::little>(Ptr);
+ // The value returned from RecordTableGenerator.Emit.
+ const uint64_t RecordTableOffset =
+ support::endian::readNext<uint64_t, llvm::endianness::little>(Ptr);
+
+ // Read the schema.
+ auto SchemaOr = memprof::readMemProfSchema(Ptr);
+ if (!SchemaOr)
+ return SchemaOr.takeError();
+ Schema = SchemaOr.get();
+
+ FrameBase = Ptr;
+ CallStackBase = Start + CallStackPayloadOffset;
+
+ // Compute the number of elements in the radix tree array. Since we use this
+ // to reserve enough bits in a BitVector, it's totally OK if we overestimate
+ // this number a little bit because of padding just before the next section.
+ RadixTreeSize = (RecordPayloadOffset - CallStackPayloadOffset) /
+ sizeof(memprof::LinearFrameId);
+
+ // Now initialize the table reader with a pointer into data buffer.
+ MemProfRecordTable.reset(MemProfRecordHashTable::Create(
+ /*Buckets=*/Start + RecordTableOffset,
+ /*Payload=*/Start + RecordPayloadOffset,
+ /*Base=*/Start, memprof::RecordLookupTrait(memprof::Version3, Schema)));
+
+ return Error::success();
+}
+
+Error IndexedMemProfReader::deserialize(const unsigned char *Start,
+ uint64_t MemProfOffset) {
+ const unsigned char *Ptr = Start + MemProfOffset;
+
+ // Read the MemProf version number.
+ const uint64_t FirstWord =
+ support::endian::readNext<uint64_t, llvm::endianness::little>(Ptr);
+
+ if (FirstWord == memprof::Version2 || FirstWord == memprof::Version3) {
+ // Everything is good. We can proceed to deserialize the rest.
+ Version = static_cast<memprof::IndexedVersion>(FirstWord);
+ } else {
+ return make_error<InstrProfError>(
+ instrprof_error::unsupported_version,
+ formatv("MemProf version {} not supported; "
+ "requires version between {} and {}, inclusive",
+ FirstWord, memprof::MinimumSupportedVersion,
+ memprof::MaximumSupportedVersion));
+ }
+
+ switch (Version) {
+ case memprof::Version2:
+ if (Error E = deserializeV2(Start, Ptr))
+ return E;
+ break;
+ case memprof::Version3:
+ if (Error E = deserializeV3(Start, Ptr))
+ return E;
+ break;
+ }
+
+ return Error::success();
+}
+
} // namespace llvm
diff --git a/llvm/lib/ProfileData/InstrProfReader.cpp b/llvm/lib/ProfileData/InstrProfReader.cpp
index 4075b513c218d..295f2a633e6c7 100644
--- a/llvm/lib/ProfileData/InstrProfReader.cpp
+++ b/llvm/lib/ProfileData/InstrProfReader.cpp
@@ -1230,129 +1230,6 @@ IndexedInstrProfReader::readSummary(IndexedInstrProf::ProfVersion Version,
}
}
-Error IndexedMemProfReader::deserializeV2(const unsigned char *Start,
- const unsigned char *Ptr) {
- // The value returned from RecordTableGenerator.Emit.
- const uint64_t RecordTableOffset =
- support::endian::readNext<uint64_t, llvm::endianness::little>(Ptr);
- // The offset in the stream right before invoking
- // FrameTableGenerator.Emit.
- const uint64_t FramePayloadOffset =
- support::endian::readNext<uint64_t, llvm::endianness::little>(Ptr);
- // The value returned from FrameTableGenerator.Emit.
- const uint64_t FrameTableOffset =
- support::endian::readNext<uint64_t, llvm::endianness::little>(Ptr);
-
- // The offset in the stream right before invoking
- // CallStackTableGenerator.Emit.
- uint64_t CallStackPayloadOffset = 0;
- // The value returned from CallStackTableGenerator.Emit.
- uint64_t CallStackTableOffset = 0;
- if (Version >= memprof::Version2) {
- CallStackPayloadOffset =
- support::endian::readNext<uint64_t, llvm::endianness::little>(Ptr);
- CallStackTableOffset =
- support::endian::readNext<uint64_t, llvm::endianness::little>(Ptr);
- }
-
- // Read the schema.
- auto SchemaOr = memprof::readMemProfSchema(Ptr);
- if (!SchemaOr)
- return SchemaOr.takeError();
- Schema = SchemaOr.get();
-
- // Now initialize the table reader with a pointer into data buffer.
- MemProfRecordTable.reset(MemProfRecordHashTable::Create(
- /*Buckets=*/Start + RecordTableOffset,
- /*Payload=*/Ptr,
- /*Base=*/Start, memprof::RecordLookupTrait(Version, Schema)));
-
- // Initialize the frame table reader with the payload and bucket offsets.
- MemProfFrameTable.reset(MemProfFrameHashTable::Create(
- /*Buckets=*/Start + FrameTableOffset,
- /*Payload=*/Start + FramePayloadOffset,
- /*Base=*/Start));
-
- if (Version >= memprof::Version2)
- MemProfCallStackTable.reset(MemProfCallStackHashTable::Create(
- /*Buckets=*/Start + CallStackTableOffset,
- /*Payload=*/Start + CallStackPayloadOffset,
- /*Base=*/Start));
-
- return Error::success();
-}
-
-Error IndexedMemProfReader::deserializeV3(const unsigned char *Start,
- const unsigned char *Ptr) {
- // The offset in the stream right before invoking
- // CallStackTableGenerator.Emit.
- const uint64_t CallStackPayloadOffset =
- support::endian::readNext<uint64_t, llvm::endianness::little>(Ptr);
- // The offset in the stream right before invoking RecordTableGenerator.Emit.
- const uint64_t RecordPayloadOffset =
- support::endian::readNext<uint64_t, llvm::endianness::little>(Ptr);
- // The value returned from RecordTableGenerator.Emit.
- const uint64_t RecordTableOffset =
- support::endian::readNext<uint64_t, llvm::endianness::little>(Ptr);
-
- // Read the schema.
- auto SchemaOr = memprof::readMemProfSchema(Ptr);
- if (!SchemaOr)
- return SchemaOr.takeError();
- Schema = SchemaOr.get();
-
- FrameBase = Ptr;
- CallStackBase = Start + CallStackPayloadOffset;
-
- // Compute the number of elements in the radix tree array. Since we use this
- // to reserve enough bits in a BitVector, it's totally OK if we overestimate
- // this number a little bit because of padding just before the next section.
- RadixTreeSize = (RecordPayloadOffset - CallStackPayloadOffset) /
- sizeof(memprof::LinearFrameId);
-
- // Now initialize the table reader with a pointer into data buffer.
- MemProfRecordTable.reset(MemProfRecordHashTable::Create(
- /*Buckets=*/Start + RecordTableOffset,
- /*Payload=*/Start + RecordPayloadOffset,
- /*Base=*/Start, memprof::RecordLookupTrait(memprof::Version3, Schema)));
-
- return Error::success();
-}
-
-Error IndexedMemProfReader::deserialize(const unsigned char *Start,
- uint64_t MemProfOffset) {
- const unsigned char *Ptr = Start + MemProfOffset;
-
- // Read the MemProf version number.
- const uint64_t FirstWord =
- support::endian::readNext<uint64_t, llvm::endianness::little>(Ptr);
-
- if (FirstWord == memprof::Version2 || FirstWord == memprof::Version3) {
- // Everything is good. We can proceed to deserialize the rest.
- Version = static_cast<memprof::IndexedVersion>(FirstWord);
- } else {
- return make_error<InstrProfError>(
- instrprof_error::unsupported_version,
- formatv("MemProf version {} not supported; "
- "requires version between {} and {}, inclusive",
- FirstWord, memprof::MinimumSupportedVersion,
- memprof::MaximumSupportedVersion));
- }
-
- switch (Version) {
- case memprof::Version2:
- if (Error E = deserializeV2(Start, Ptr))
- return E;
- break;
- case memprof::Version3:
- if (Error E = deserializeV3(Start, Ptr))
- return E;
- break;
- }
-
- return Error::success();
-}
-
Error IndexedInstrProfReader::readHeader() {
using namespace support;
|
teresajohnson
approved these changes
Apr 24, 2025
mingmingl-llvm
approved these changes
Apr 24, 2025
Thank you for spotting it! Fixed. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/11/builds/13657 Here is the relevant piece of the build log for the reference |
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
….cpp (NFC) (llvm#137089) This patch moves IndexedMemProfReader::deserialize and its subroutines to IndexedMemProfData.cpp, building on: commit 9a8f90d Author: Kazu Hirata <[email protected]> Date: Wed Apr 23 15:39:45 2025 -0700 The intent is as follows: - Reduce the size of InstrProfReader.cpp. - Move the subroutines to a separate file because they don't interact with anything else in InstrProfReader.cpp.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
….cpp (NFC) (llvm#137089) This patch moves IndexedMemProfReader::deserialize and its subroutines to IndexedMemProfData.cpp, building on: commit 9a8f90d Author: Kazu Hirata <[email protected]> Date: Wed Apr 23 15:39:45 2025 -0700 The intent is as follows: - Reduce the size of InstrProfReader.cpp. - Move the subroutines to a separate file because they don't interact with anything else in InstrProfReader.cpp.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
….cpp (NFC) (llvm#137089) This patch moves IndexedMemProfReader::deserialize and its subroutines to IndexedMemProfData.cpp, building on: commit 9a8f90d Author: Kazu Hirata <[email protected]> Date: Wed Apr 23 15:39:45 2025 -0700 The intent is as follows: - Reduce the size of InstrProfReader.cpp. - Move the subroutines to a separate file because they don't interact with anything else in InstrProfReader.cpp.
Ankur-0429
pushed a commit
to Ankur-0429/llvm-project
that referenced
this pull request
May 9, 2025
….cpp (NFC) (llvm#137089) This patch moves IndexedMemProfReader::deserialize and its subroutines to IndexedMemProfData.cpp, building on: commit 9a8f90d Author: Kazu Hirata <[email protected]> Date: Wed Apr 23 15:39:45 2025 -0700 The intent is as follows: - Reduce the size of InstrProfReader.cpp. - Move the subroutines to a separate file because they don't interact with anything else in InstrProfReader.cpp.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 moves IndexedMemProfReader::deserialize and its subroutines
to IndexedMemProfData.cpp, building on:
commit 9a8f90d
Author: Kazu Hirata [email protected]
Date: Wed Apr 23 15:39:45 2025 -0700
The intent is as follows:
with anything else in InstrProfReader.cpp.