Skip to content

Commit f2d22b5

Browse files
[memprof] Make RecordWriterTrait a non-template class (#87604)
commit d89914f Author: Kazu Hirata <[email protected]> Date: Wed Apr 3 21:48:38 2024 -0700 changed RecordWriterTrait to a template class with IndexedVersion as a template parameter. This patch changes the class back to a non-template one while retaining the ability to serialize multiple versions. The reason I changed RecordWriterTrait to a template class was because, even if RecordWriterTrait had IndexedVersion as a member variable, RecordWriterTrait::EmitKeyDataLength, being a static function, would not have access to the variable. Since OnDiskChainedHashTableGenerator calls EmitKeyDataLength as: const std::pair<offset_type, offset_type> &Len = InfoObj.EmitKeyDataLength(Out, I->Key, I->Data); we can make EmitKeyDataLength a member function, but we have one problem. InstrProfWriter::writeImpl calls: void insert(typename Info::key_type_ref Key, typename Info::data_type_ref Data) { Info InfoObj; insert(Key, Data, InfoObj); } which default-constructs RecordWriterTrait without a specific version number. This patch fixes the problem by adjusting InstrProfWriter::writeImpl to call the other form of insert instead: void insert(typename Info::key_type_ref Key, typename Info::data_type_ref Data, Info &InfoObj) To prevent an accidental invocation of the default constructor of RecordWriterTrait, this patch deletes the default constructor.
1 parent 258dd64 commit f2d22b5

File tree

2 files changed

+10
-7
lines changed

2 files changed

+10
-7
lines changed

llvm/include/llvm/ProfileData/MemProf.h

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,7 @@ class RecordLookupTrait {
502502
};
503503

504504
// Trait for writing IndexedMemProfRecord data to the on-disk hash table.
505-
template <IndexedVersion Version> class RecordWriterTrait {
505+
class RecordWriterTrait {
506506
public:
507507
using key_type = uint64_t;
508508
using key_type_ref = uint64_t;
@@ -517,12 +517,16 @@ template <IndexedVersion Version> class RecordWriterTrait {
517517
// we must use a default constructor with no params for the writer trait so we
518518
// have a public member which must be initialized by the user.
519519
MemProfSchema *Schema = nullptr;
520+
// The MemProf version to use for the serialization.
521+
IndexedVersion Version;
520522

521-
RecordWriterTrait() = default;
523+
// We do not support the default constructor, which does not set Version.
524+
RecordWriterTrait() = delete;
525+
RecordWriterTrait(IndexedVersion V) : Version(V) {}
522526

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

525-
static std::pair<offset_type, offset_type>
529+
std::pair<offset_type, offset_type>
526530
EmitKeyDataLength(raw_ostream &Out, key_type_ref K, data_type_ref V) {
527531
using namespace support;
528532

llvm/lib/ProfileData/InstrProfWriter.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -558,14 +558,13 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
558558
}
559559

560560
auto RecordWriter =
561-
std::make_unique<memprof::RecordWriterTrait<memprof::Version1>>();
561+
std::make_unique<memprof::RecordWriterTrait>(memprof::Version1);
562562
RecordWriter->Schema = &Schema;
563-
OnDiskChainedHashTableGenerator<
564-
memprof::RecordWriterTrait<memprof::Version1>>
563+
OnDiskChainedHashTableGenerator<memprof::RecordWriterTrait>
565564
RecordTableGenerator;
566565
for (auto &I : MemProfRecordData) {
567566
// Insert the key (func hash) and value (memprof record).
568-
RecordTableGenerator.insert(I.first, I.second);
567+
RecordTableGenerator.insert(I.first, I.second, *RecordWriter.get());
569568
}
570569
// Release the memory of this MapVector as it is no longer needed.
571570
MemProfRecordData.clear();

0 commit comments

Comments
 (0)