Skip to content

Commit 8ad980d

Browse files
[memprof] Refactor getMemProfRecord (NFC) (#93138)
This patch refactors getMemProfRecord for readability while adding consistency checks. - This patch adds a switch statement on the MemProf version just like most places dealing with MemProf serialization/deserialization. - This patch adds asserts to ensure that the exact set of data structures are available while ones we do not use are not present. That is, getMemProfRecord no longer determines the version based on the availability of MemProfCallStackTable.
1 parent 87452bc commit 8ad980d

File tree

2 files changed

+69
-33
lines changed

2 files changed

+69
-33
lines changed

llvm/include/llvm/ProfileData/InstrProfReader.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -649,6 +649,8 @@ class InstrProfReaderRemapper {
649649

650650
class IndexedMemProfReader {
651651
private:
652+
/// The MemProf version.
653+
memprof::IndexedVersion Version = memprof::Version0;
652654
/// MemProf profile schema (if available).
653655
memprof::MemProfSchema Schema;
654656
/// MemProf record profile data on-disk indexed via llvm::md5(FunctionName).

llvm/lib/ProfileData/InstrProfReader.cpp

Lines changed: 67 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1212,7 +1212,6 @@ Error IndexedMemProfReader::deserialize(const unsigned char *Start,
12121212
const uint64_t FirstWord =
12131213
support::endian::readNext<uint64_t, llvm::endianness::little>(Ptr);
12141214

1215-
memprof::IndexedVersion Version = memprof::Version0;
12161215
if (FirstWord == memprof::Version1 || FirstWord == memprof::Version2) {
12171216
// Everything is good. We can proceed to deserialize the rest.
12181217
Version = static_cast<memprof::IndexedVersion>(FirstWord);
@@ -1490,6 +1489,55 @@ Expected<InstrProfRecord> IndexedInstrProfReader::getInstrProfRecord(
14901489
return error(instrprof_error::unknown_function);
14911490
}
14921491

1492+
static Expected<memprof::MemProfRecord>
1493+
getMemProfRecordV0(const memprof::IndexedMemProfRecord &IndexedRecord,
1494+
MemProfFrameHashTable &MemProfFrameTable) {
1495+
memprof::FrameIdConverter<MemProfFrameHashTable> FrameIdConv(
1496+
MemProfFrameTable);
1497+
1498+
memprof::MemProfRecord Record =
1499+
memprof::MemProfRecord(IndexedRecord, FrameIdConv);
1500+
1501+
// Check that all frame ids were successfully converted to frames.
1502+
if (FrameIdConv.LastUnmappedId) {
1503+
return make_error<InstrProfError>(instrprof_error::hash_mismatch,
1504+
"memprof frame not found for frame id " +
1505+
Twine(*FrameIdConv.LastUnmappedId));
1506+
}
1507+
1508+
return Record;
1509+
}
1510+
1511+
static Expected<memprof::MemProfRecord>
1512+
getMemProfRecordV2(const memprof::IndexedMemProfRecord &IndexedRecord,
1513+
MemProfFrameHashTable &MemProfFrameTable,
1514+
MemProfCallStackHashTable &MemProfCallStackTable) {
1515+
memprof::FrameIdConverter<MemProfFrameHashTable> FrameIdConv(
1516+
MemProfFrameTable);
1517+
1518+
memprof::CallStackIdConverter<MemProfCallStackHashTable> CSIdConv(
1519+
MemProfCallStackTable, FrameIdConv);
1520+
1521+
memprof::MemProfRecord Record = IndexedRecord.toMemProfRecord(CSIdConv);
1522+
1523+
// Check that all call stack ids were successfully converted to call stacks.
1524+
if (CSIdConv.LastUnmappedId) {
1525+
return make_error<InstrProfError>(
1526+
instrprof_error::hash_mismatch,
1527+
"memprof call stack not found for call stack id " +
1528+
Twine(*CSIdConv.LastUnmappedId));
1529+
}
1530+
1531+
// Check that all frame ids were successfully converted to frames.
1532+
if (FrameIdConv.LastUnmappedId) {
1533+
return make_error<InstrProfError>(instrprof_error::hash_mismatch,
1534+
"memprof frame not found for frame id " +
1535+
Twine(*FrameIdConv.LastUnmappedId));
1536+
}
1537+
1538+
return Record;
1539+
}
1540+
14931541
Expected<memprof::MemProfRecord>
14941542
IndexedMemProfReader::getMemProfRecord(const uint64_t FuncNameHash) const {
14951543
// TODO: Add memprof specific errors.
@@ -1502,41 +1550,27 @@ IndexedMemProfReader::getMemProfRecord(const uint64_t FuncNameHash) const {
15021550
instrprof_error::unknown_function,
15031551
"memprof record not found for function hash " + Twine(FuncNameHash));
15041552

1505-
// Setup a callback to convert from frame ids to frame using the on-disk
1506-
// FrameData hash table.
1507-
memprof::FrameIdConverter<MemProfFrameHashTable> FrameIdConv(
1508-
*MemProfFrameTable.get());
1509-
15101553
const memprof::IndexedMemProfRecord IndexedRecord = *Iter;
1511-
memprof::MemProfRecord Record;
1512-
if (MemProfCallStackTable) {
1513-
// Setup a callback to convert call stack ids to call stacks using the
1514-
// on-disk hash table.
1515-
memprof::CallStackIdConverter<MemProfCallStackHashTable> CSIdConv(
1516-
*MemProfCallStackTable.get(), FrameIdConv);
1517-
1518-
Record = IndexedRecord.toMemProfRecord(CSIdConv);
1519-
1520-
// Check that all call stack ids were successfully converted to call stacks.
1521-
if (CSIdConv.LastUnmappedId) {
1522-
return make_error<InstrProfError>(
1523-
instrprof_error::hash_mismatch,
1524-
"memprof call stack not found for call stack id " +
1525-
Twine(*CSIdConv.LastUnmappedId));
1526-
}
1527-
} else {
1528-
Record = memprof::MemProfRecord(IndexedRecord, FrameIdConv);
1554+
switch (Version) {
1555+
case memprof::Version0:
1556+
case memprof::Version1:
1557+
assert(MemProfFrameTable && "MemProfFrameTable must be available");
1558+
assert(!MemProfCallStackTable &&
1559+
"MemProfCallStackTable must not be available");
1560+
return getMemProfRecordV0(IndexedRecord, *MemProfFrameTable);
1561+
case memprof::Version2:
1562+
assert(MemProfFrameTable && "MemProfFrameTable must be available");
1563+
assert(MemProfCallStackTable && "MemProfCallStackTable must be available");
1564+
return getMemProfRecordV2(IndexedRecord, *MemProfFrameTable,
1565+
*MemProfCallStackTable);
15291566
}
15301567

1531-
// Check that all frame ids were successfully converted to frames.
1532-
if (FrameIdConv.LastUnmappedId) {
1533-
return make_error<InstrProfError>(
1534-
instrprof_error::hash_mismatch,
1535-
"memprof frame not found for frame id " +
1536-
Twine(*FrameIdConv.LastUnmappedId));
1537-
}
1538-
1539-
return Record;
1568+
return make_error<InstrProfError>(
1569+
instrprof_error::unsupported_version,
1570+
formatv("MemProf version {} not supported; "
1571+
"requires version between {} and {}, inclusive",
1572+
Version, memprof::MinimumSupportedVersion,
1573+
memprof::MaximumSupportedVersion));
15401574
}
15411575

15421576
Error IndexedInstrProfReader::getFunctionCounts(StringRef FuncName,

0 commit comments

Comments
 (0)