Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[memprof] Omit the key/data lengths for the frame table #89711

Closed
wants to merge 1 commit into from
Closed

[memprof] Omit the key/data lengths for the frame table #89711

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 29 additions & 7 deletions llvm/include/llvm/ProfileData/MemProf.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -578,14 +578,21 @@ class FrameWriterTrait {

static hash_value_type ComputeHash(key_type_ref K) { return K; }

static std::pair<offset_type, offset_type>
FrameWriterTrait() = delete;
FrameWriterTrait(IndexedVersion Version) : Version(Version) {}

std::pair<offset_type, offset_type>
EmitKeyDataLength(raw_ostream &Out, key_type_ref K, data_type_ref V) {
using namespace support;
endian::Writer LE(Out, llvm::endianness::little);
offset_type N = sizeof(K);
LE.write<offset_type>(N);
offset_type M = V.serializedSize();
LE.write<offset_type>(M);
offset_type M = Frame::serializedSize();
// Starting with Version2, we do not explicitly emit the key/data lengths
// because they are constants.
if (Version < Version2) {
LE.write<offset_type>(N);
LE.write<offset_type>(M);
}
return std::make_pair(N, M);
}

Expand All @@ -599,6 +606,10 @@ class FrameWriterTrait {
offset_type /*Unused*/) {
V.serialize(Out);
}

private:
// Holds the MemProf version.
IndexedVersion Version;
};

// Trait for reading frame mappings from the on-disk hash table.
Expand All @@ -610,6 +621,9 @@ class FrameLookupTrait {
using hash_value_type = FrameId;
using offset_type = uint64_t;

FrameLookupTrait() = delete;
FrameLookupTrait(IndexedVersion Version) : Version(Version) {}

static bool EqualKey(internal_key_type A, internal_key_type B) {
return A == B;
}
Expand All @@ -618,14 +632,18 @@ class FrameLookupTrait {

hash_value_type ComputeHash(internal_key_type K) { return K; }

static std::pair<offset_type, offset_type>
std::pair<offset_type, offset_type>
ReadKeyDataLength(const unsigned char *&D) {
using namespace support;

offset_type KeyLen =
endian::readNext<offset_type, llvm::endianness::little>(D);
Version < Version2
? endian::readNext<offset_type, llvm::endianness::little>(D)
: sizeof(FrameId);
offset_type DataLen =
endian::readNext<offset_type, llvm::endianness::little>(D);
Version < Version2
? endian::readNext<offset_type, llvm::endianness::little>(D)
: Frame::serializedSize();
return std::make_pair(KeyLen, DataLen);
}

Expand All @@ -638,6 +656,10 @@ class FrameLookupTrait {
offset_type /*Unused*/) {
return Frame::deserialize(D);
}

private:
// Holds the MemProf version.
IndexedVersion Version;
};

// Trait for writing call stacks to the on-disk hash table.
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/ProfileData/InstrProfReader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1272,7 +1272,7 @@ Error IndexedMemProfReader::deserialize(const unsigned char *Start,
MemProfFrameTable.reset(MemProfFrameHashTable::Create(
/*Buckets=*/Start + FrameTableOffset,
/*Payload=*/Start + FramePayloadOffset,
/*Base=*/Start));
/*Base=*/Start, memprof::FrameLookupTrait(Version)));

if (Version >= memprof::Version2)
MemProfCallStackTable.reset(MemProfCallStackHashTable::Create(
Expand Down
17 changes: 11 additions & 6 deletions llvm/lib/ProfileData/InstrProfWriter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,17 +470,19 @@ static uint64_t writeMemProfRecords(
// Serialize MemProfFrameData. Return FrameTableOffset.
static uint64_t writeMemProfFrames(
ProfOStream &OS,
llvm::MapVector<memprof::FrameId, memprof::Frame> &MemProfFrameData) {
llvm::MapVector<memprof::FrameId, memprof::Frame> &MemProfFrameData,
memprof::IndexedVersion Version) {
memprof::FrameWriterTrait FrameWriter(Version);
OnDiskChainedHashTableGenerator<memprof::FrameWriterTrait>
FrameTableGenerator;
for (auto &[FrameId, Frame] : MemProfFrameData) {
// Insert the key (frame id) and value (frame contents).
FrameTableGenerator.insert(FrameId, Frame);
FrameTableGenerator.insert(FrameId, Frame, FrameWriter);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there changes needed in this patch to the insert() and Emit() methods of OnDiskChainedHashTableGenerator to take the writer trait?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there changes needed in this patch to the insert() and Emit() methods of OnDiskChainedHashTableGenerator to take the writer trait?

Yes, FrameWriter carries the version, so we need to pass FrameWriter to both insert and Emit.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I think that part is missing from the PR?

}
// Release the memory of this MapVector as it is no longer needed.
MemProfFrameData.clear();

return FrameTableGenerator.Emit(OS.OS);
return FrameTableGenerator.Emit(OS.OS, FrameWriter);
}

static uint64_t writeMemProfCallStacks(
Expand Down Expand Up @@ -514,7 +516,8 @@ static Error writeMemProfV0(
writeMemProfRecords(OS, MemProfRecordData, &Schema, memprof::Version0);

uint64_t FramePayloadOffset = OS.tell();
uint64_t FrameTableOffset = writeMemProfFrames(OS, MemProfFrameData);
uint64_t FrameTableOffset =
writeMemProfFrames(OS, MemProfFrameData, memprof::Version0);

uint64_t Header[] = {RecordTableOffset, FramePayloadOffset, FrameTableOffset};
OS.patch({{HeaderUpdatePos, Header, std::size(Header)}});
Expand All @@ -540,7 +543,8 @@ static Error writeMemProfV1(
writeMemProfRecords(OS, MemProfRecordData, &Schema, memprof::Version1);

uint64_t FramePayloadOffset = OS.tell();
uint64_t FrameTableOffset = writeMemProfFrames(OS, MemProfFrameData);
uint64_t FrameTableOffset =
writeMemProfFrames(OS, MemProfFrameData, memprof::Version1);

uint64_t Header[] = {RecordTableOffset, FramePayloadOffset, FrameTableOffset};
OS.patch({{HeaderUpdatePos, Header, std::size(Header)}});
Expand Down Expand Up @@ -570,7 +574,8 @@ static Error writeMemProfV2(
writeMemProfRecords(OS, MemProfRecordData, &Schema, memprof::Version2);

uint64_t FramePayloadOffset = OS.tell();
uint64_t FrameTableOffset = writeMemProfFrames(OS, MemProfFrameData);
uint64_t FrameTableOffset =
writeMemProfFrames(OS, MemProfFrameData, memprof::Version2);

uint64_t CallStackPayloadOffset = OS.tell();
uint64_t CallStackTableOffset =
Expand Down
Loading