Skip to content

Commit 81c5f65

Browse files
author
Snehasish Kumar
committed
[MemProf] Extend CallSite information to include potential callees.
* Added YAML traits for `CallSiteInfo` * Updated the `MemProfReader` to pass `Frames` instead of the entire `CallSiteInfo` * Updated test cases to use `testing::Field` * Add YAML sequence traits for CallSiteInfo in MemProfYAML
1 parent 4d59552 commit 81c5f65

File tree

6 files changed

+137
-45
lines changed

6 files changed

+137
-45
lines changed

‎llvm/include/llvm/ProfileData/MemProf.h‎

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,28 @@ using CallStackId = uint64_t;
342342
// A type representing the index into the call stack array.
343343
using LinearCallStackId = uint32_t;
344344

345+
// Holds call site information with indexed frame contents.
346+
struct IndexedCallSiteInfo {
347+
// The call stack ID for this call site
348+
CallStackId CSId = 0;
349+
// The GUIDs of the callees at this call site
350+
std::vector<GlobalValue::GUID> CalleeGuids;
351+
352+
IndexedCallSiteInfo() = default;
353+
IndexedCallSiteInfo(CallStackId CSId) : CSId(CSId) {}
354+
IndexedCallSiteInfo(CallStackId CSId,
355+
std::vector<GlobalValue::GUID> CalleeGuids)
356+
: CSId(CSId), CalleeGuids(std::move(CalleeGuids)) {}
357+
358+
bool operator==(const IndexedCallSiteInfo &Other) const {
359+
return CSId == Other.CSId && CalleeGuids == Other.CalleeGuids;
360+
}
361+
362+
bool operator!=(const IndexedCallSiteInfo &Other) const {
363+
return !operator==(Other);
364+
}
365+
};
366+
345367
// Holds allocation information in a space efficient format where frames are
346368
// represented using unique identifiers.
347369
struct IndexedAllocationInfo {
@@ -410,7 +432,7 @@ struct IndexedMemProfRecord {
410432
// list of inline locations in bottom-up order i.e. from leaf to root. The
411433
// inline location list may include additional entries, users should pick
412434
// the last entry in the list with the same function GUID.
413-
llvm::SmallVector<CallStackId> CallSiteIds;
435+
llvm::SmallVector<IndexedCallSiteInfo> CallSites;
414436

415437
void clear() { *this = IndexedMemProfRecord(); }
416438

@@ -427,7 +449,7 @@ struct IndexedMemProfRecord {
427449
if (Other.AllocSites != AllocSites)
428450
return false;
429451

430-
if (Other.CallSiteIds != CallSiteIds)
452+
if (Other.CallSites != CallSites)
431453
return false;
432454
return true;
433455
}
@@ -455,14 +477,37 @@ struct IndexedMemProfRecord {
455477
static GlobalValue::GUID getGUID(const StringRef FunctionName);
456478
};
457479

480+
// Holds call site information with frame contents inline.
481+
struct CallSiteInfo {
482+
// The frames in the call stack
483+
std::vector<Frame> Frames;
484+
485+
// The GUIDs of the callees at this call site
486+
std::vector<GlobalValue::GUID> CalleeGuids;
487+
488+
CallSiteInfo() = default;
489+
CallSiteInfo(std::vector<Frame> Frames) : Frames(std::move(Frames)) {}
490+
CallSiteInfo(std::vector<Frame> Frames,
491+
std::vector<GlobalValue::GUID> CalleeGuids)
492+
: Frames(std::move(Frames)), CalleeGuids(std::move(CalleeGuids)) {}
493+
494+
bool operator==(const CallSiteInfo &Other) const {
495+
return Frames == Other.Frames && CalleeGuids == Other.CalleeGuids;
496+
}
497+
498+
bool operator!=(const CallSiteInfo &Other) const {
499+
return !operator==(Other);
500+
}
501+
};
502+
458503
// Holds the memprof profile information for a function. The internal
459504
// representation stores frame contents inline. This representation should
460505
// be used for small amount of temporary, in memory instances.
461506
struct MemProfRecord {
462507
// Same as IndexedMemProfRecord::AllocSites with frame contents inline.
463508
llvm::SmallVector<AllocationInfo> AllocSites;
464509
// Same as IndexedMemProfRecord::CallSites with frame contents inline.
465-
llvm::SmallVector<std::vector<Frame>> CallSites;
510+
llvm::SmallVector<CallSiteInfo> CallSites;
466511

467512
MemProfRecord() = default;
468513

@@ -476,8 +521,8 @@ struct MemProfRecord {
476521

477522
if (!CallSites.empty()) {
478523
OS << " CallSites:\n";
479-
for (const std::vector<Frame> &Frames : CallSites) {
480-
for (const Frame &F : Frames) {
524+
for (const CallSiteInfo &CS : CallSites) {
525+
for (const Frame &F : CS.Frames) {
481526
OS << " -\n";
482527
F.printYAML(OS);
483528
}

‎llvm/include/llvm/ProfileData/MemProfYAML.h‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,28 @@ template <> struct MappingTraits<memprof::AllocationInfo> {
155155
// In YAML, we use GUIDMemProfRecordPair instead of MemProfRecord so that we can
156156
// treat the GUID and the fields within MemProfRecord at the same level as if
157157
// the GUID were part of MemProfRecord.
158+
template <> struct MappingTraits<memprof::CallSiteInfo> {
159+
static void mapping(IO &Io, memprof::CallSiteInfo &CS) {
160+
Io.mapRequired("Frames", CS.Frames);
161+
Io.mapOptional("CalleeGuids", CS.CalleeGuids);
162+
}
163+
};
164+
165+
// Add sequence traits for SmallVector<CallSiteInfo>
166+
template <unsigned N>
167+
struct SequenceTraits<SmallVector<memprof::CallSiteInfo, N>> {
168+
static size_t size(IO &IO, SmallVector<memprof::CallSiteInfo, N> &Seq) {
169+
return Seq.size();
170+
}
171+
172+
static memprof::CallSiteInfo &
173+
element(IO &IO, SmallVector<memprof::CallSiteInfo, N> &Seq, size_t Index) {
174+
if (Index >= Seq.size())
175+
Seq.resize(Index + 1);
176+
return Seq[Index];
177+
}
178+
};
179+
158180
template <> struct MappingTraits<memprof::GUIDMemProfRecordPair> {
159181
static void mapping(IO &Io, memprof::GUIDMemProfRecordPair &Pair) {
160182
Io.mapRequired("GUID", Pair.GUID);
@@ -174,6 +196,7 @@ template <> struct MappingTraits<memprof::AllMemProfData> {
174196
LLVM_YAML_IS_SEQUENCE_VECTOR(memprof::Frame)
175197
LLVM_YAML_IS_SEQUENCE_VECTOR(std::vector<memprof::Frame>)
176198
LLVM_YAML_IS_SEQUENCE_VECTOR(memprof::AllocationInfo)
199+
LLVM_YAML_IS_SEQUENCE_VECTOR(memprof::CallSiteInfo)
177200
LLVM_YAML_IS_SEQUENCE_VECTOR(memprof::GUIDMemProfRecordPair)
178201

179202
#endif // LLVM_PROFILEDATA_MEMPROFYAML_H_

‎llvm/lib/ProfileData/MemProf.cpp‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ static size_t serializedSizeV2(const IndexedMemProfRecord &Record,
6464
// The number of callsites we have information for.
6565
Result += sizeof(uint64_t);
6666
// The CallStackId
67-
Result += Record.CallSiteIds.size() * sizeof(CallStackId);
67+
Result += Record.CallSites.size() * sizeof(CallStackId);
6868
return Result;
6969
}
7070

@@ -78,7 +78,7 @@ static size_t serializedSizeV3(const IndexedMemProfRecord &Record,
7878
// The number of callsites we have information for.
7979
Result += sizeof(uint64_t);
8080
// The linear call stack ID.
81-
Result += Record.CallSiteIds.size() * sizeof(LinearCallStackId);
81+
Result += Record.CallSites.size() * sizeof(LinearCallStackId);
8282
return Result;
8383
}
8484

@@ -106,9 +106,9 @@ static void serializeV2(const IndexedMemProfRecord &Record,
106106
}
107107

108108
// Related contexts.
109-
LE.write<uint64_t>(Record.CallSiteIds.size());
110-
for (const auto &CSId : Record.CallSiteIds)
111-
LE.write<CallStackId>(CSId);
109+
LE.write<uint64_t>(Record.CallSites.size());
110+
for (const auto &CS : Record.CallSites)
111+
LE.write<CallStackId>(CS.CSId);
112112
}
113113

114114
static void serializeV3(
@@ -127,10 +127,10 @@ static void serializeV3(
127127
}
128128

129129
// Related contexts.
130-
LE.write<uint64_t>(Record.CallSiteIds.size());
131-
for (const auto &CSId : Record.CallSiteIds) {
132-
assert(MemProfCallStackIndexes.contains(CSId));
133-
LE.write<LinearCallStackId>(MemProfCallStackIndexes[CSId]);
130+
LE.write<uint64_t>(Record.CallSites.size());
131+
for (const auto &CS : Record.CallSites) {
132+
assert(MemProfCallStackIndexes.contains(CS.CSId));
133+
LE.write<LinearCallStackId>(MemProfCallStackIndexes[CS.CSId]);
134134
}
135135
}
136136

@@ -170,11 +170,11 @@ static IndexedMemProfRecord deserializeV2(const MemProfSchema &Schema,
170170
// Read the callsite information.
171171
const uint64_t NumCtxs =
172172
endian::readNext<uint64_t, llvm::endianness::little>(Ptr);
173-
Record.CallSiteIds.reserve(NumCtxs);
173+
Record.CallSites.reserve(NumCtxs);
174174
for (uint64_t J = 0; J < NumCtxs; J++) {
175175
CallStackId CSId =
176176
endian::readNext<CallStackId, llvm::endianness::little>(Ptr);
177-
Record.CallSiteIds.push_back(CSId);
177+
Record.CallSites.push_back(IndexedCallSiteInfo(CSId));
178178
}
179179

180180
return Record;
@@ -202,15 +202,15 @@ static IndexedMemProfRecord deserializeV3(const MemProfSchema &Schema,
202202
// Read the callsite information.
203203
const uint64_t NumCtxs =
204204
endian::readNext<uint64_t, llvm::endianness::little>(Ptr);
205-
Record.CallSiteIds.reserve(NumCtxs);
205+
Record.CallSites.reserve(NumCtxs);
206206
for (uint64_t J = 0; J < NumCtxs; J++) {
207207
// We are storing LinearCallStackId in CallSiteIds, which is a vector of
208208
// CallStackId. Assert that CallStackId is no smaller than
209209
// LinearCallStackId.
210210
static_assert(sizeof(LinearCallStackId) <= sizeof(CallStackId));
211211
LinearCallStackId CSId =
212212
endian::readNext<LinearCallStackId, llvm::endianness::little>(Ptr);
213-
Record.CallSiteIds.push_back(CSId);
213+
Record.CallSites.push_back(IndexedCallSiteInfo(CSId));
214214
}
215215

216216
return Record;
@@ -241,9 +241,11 @@ MemProfRecord IndexedMemProfRecord::toMemProfRecord(
241241
Record.AllocSites.push_back(std::move(AI));
242242
}
243243

244-
Record.CallSites.reserve(CallSiteIds.size());
245-
for (CallStackId CSId : CallSiteIds)
246-
Record.CallSites.push_back(Callback(CSId));
244+
Record.CallSites.reserve(CallSites.size());
245+
for (const IndexedCallSiteInfo &CS : CallSites) {
246+
std::vector<Frame> Frames = Callback(CS.CSId);
247+
Record.CallSites.push_back(CallSiteInfo(std::move(Frames), CS.CalleeGuids));
248+
}
247249

248250
return Record;
249251
}

‎llvm/lib/ProfileData/MemProfReader.cpp‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -521,7 +521,8 @@ Error RawMemProfReader::mapRawProfileToRecords() {
521521
// we insert a new entry for callsite data if we need to.
522522
IndexedMemProfRecord &Record = MemProfData.Records[Id];
523523
for (LocationPtr Loc : Locs)
524-
Record.CallSiteIds.push_back(MemProfData.addCallStack(*Loc));
524+
Record.CallSites.push_back(
525+
IndexedCallSiteInfo(MemProfData.addCallStack(*Loc)));
525526
}
526527

527528
return Error::success();
@@ -808,10 +809,10 @@ void YAMLMemProfReader::parse(StringRef YAMLData) {
808809
IndexedRecord.AllocSites.emplace_back(CSId, AI.Info);
809810
}
810811

811-
// Populate CallSiteIds.
812+
// Populate CallSites with CalleeGuids.
812813
for (const auto &CallSite : Record.CallSites) {
813-
CallStackId CSId = AddCallStack(CallSite);
814-
IndexedRecord.CallSiteIds.push_back(CSId);
814+
CallStackId CSId = AddCallStack(CallSite.Frames);
815+
IndexedRecord.CallSites.emplace_back(CSId, CallSite.CalleeGuids);
815816
}
816817

817818
MemProfData.Records.try_emplace(GUID, std::move(IndexedRecord));

‎llvm/unittests/ProfileData/InstrProfTest.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ makeRecordV2(std::initializer_list<::llvm::memprof::CallStackId> AllocFrames,
397397
for (const auto &CSId : AllocFrames)
398398
MR.AllocSites.emplace_back(CSId, Block, Schema);
399399
for (const auto &CSId : CallSiteFrames)
400-
MR.CallSiteIds.push_back(CSId);
400+
MR.CallSites.push_back(llvm::memprof::IndexedCallSiteInfo(CSId));
401401
return MR;
402402
}
403403

‎llvm/unittests/ProfileData/MemProfTest.cpp‎

Lines changed: 40 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -210,8 +210,10 @@ TEST(MemProf, FillsValue) {
210210
FrameContains("abc", 5U, 30U, false));
211211

212212
EXPECT_THAT(Bar.CallSites,
213-
ElementsAre(ElementsAre(FrameContains("foo", 5U, 30U, true),
214-
FrameContains("bar", 51U, 20U, false))));
213+
ElementsAre(testing::Field(
214+
&CallSiteInfo::Frames,
215+
ElementsAre(FrameContains("foo", 5U, 30U, true),
216+
FrameContains("bar", 51U, 20U, false)))));
215217

216218
// Check the memprof record for xyz.
217219
const llvm::GlobalValue::GUID XyzId = IndexedMemProfRecord::getGUID("xyz");
@@ -220,17 +222,21 @@ TEST(MemProf, FillsValue) {
220222
// Expect the entire frame even though in practice we only need the first
221223
// entry here.
222224
EXPECT_THAT(Xyz.CallSites,
223-
ElementsAre(ElementsAre(FrameContains("xyz", 5U, 30U, true),
224-
FrameContains("abc", 5U, 30U, false))));
225+
ElementsAre(testing::Field(
226+
&CallSiteInfo::Frames,
227+
ElementsAre(FrameContains("xyz", 5U, 30U, true),
228+
FrameContains("abc", 5U, 30U, false)))));
225229

226230
// Check the memprof record for abc.
227231
const llvm::GlobalValue::GUID AbcId = IndexedMemProfRecord::getGUID("abc");
228232
ASSERT_TRUE(Records.contains(AbcId));
229233
const MemProfRecord &Abc = Records[AbcId];
230234
EXPECT_TRUE(Abc.AllocSites.empty());
231235
EXPECT_THAT(Abc.CallSites,
232-
ElementsAre(ElementsAre(FrameContains("xyz", 5U, 30U, true),
233-
FrameContains("abc", 5U, 30U, false))));
236+
ElementsAre(testing::Field(
237+
&CallSiteInfo::Frames,
238+
ElementsAre(FrameContains("xyz", 5U, 30U, true),
239+
FrameContains("abc", 5U, 30U, false)))));
234240
}
235241

236242
TEST(MemProf, PortableWrapper) {
@@ -273,7 +279,8 @@ TEST(MemProf, RecordSerializationRoundTripVerion2) {
273279
// Use the same info block for both allocation sites.
274280
Record.AllocSites.emplace_back(CSId, Info);
275281
}
276-
Record.CallSiteIds.assign(CallSiteIds);
282+
for (auto CSId : CallSiteIds)
283+
Record.CallSites.push_back(IndexedCallSiteInfo(CSId));
277284

278285
std::string Buffer;
279286
llvm::raw_string_ostream OS(Buffer);
@@ -303,7 +310,8 @@ TEST(MemProf, RecordSerializationRoundTripVersion2HotColdSchema) {
303310
// Use the same info block for both allocation sites.
304311
Record.AllocSites.emplace_back(CSId, Info, Schema);
305312
}
306-
Record.CallSiteIds.assign(CallSiteIds);
313+
for (auto CSId : CallSiteIds)
314+
Record.CallSites.push_back(IndexedCallSiteInfo(CSId));
307315

308316
std::bitset<llvm::to_underlying(Meta::Size)> SchemaBitSet;
309317
for (auto Id : Schema)
@@ -498,8 +506,8 @@ TEST(MemProf, IndexedMemProfRecordToMemProfRecord) {
498506
IndexedRecord.AllocSites.push_back(AI);
499507
AI.CSId = CS2Id;
500508
IndexedRecord.AllocSites.push_back(AI);
501-
IndexedRecord.CallSiteIds.push_back(CS3Id);
502-
IndexedRecord.CallSiteIds.push_back(CS4Id);
509+
IndexedRecord.CallSites.push_back(IndexedCallSiteInfo(CS3Id));
510+
IndexedRecord.CallSites.push_back(IndexedCallSiteInfo(CS4Id));
503511

504512
IndexedCallstackIdConveter CSIdConv(MemProfData);
505513

@@ -513,8 +521,9 @@ TEST(MemProf, IndexedMemProfRecordToMemProfRecord) {
513521
ASSERT_THAT(Record.AllocSites, SizeIs(2));
514522
EXPECT_THAT(Record.AllocSites[0].CallStack, ElementsAre(F1, F2));
515523
EXPECT_THAT(Record.AllocSites[1].CallStack, ElementsAre(F1, F3));
516-
EXPECT_THAT(Record.CallSites,
517-
ElementsAre(ElementsAre(F2, F3), ElementsAre(F2, F4)));
524+
ASSERT_THAT(Record.CallSites, SizeIs(2));
525+
EXPECT_THAT(Record.CallSites[0].Frames, ElementsAre(F2, F3));
526+
EXPECT_THAT(Record.CallSites[1].Frames, ElementsAre(F2, F4));
518527
}
519528

520529
// Populate those fields returned by getHotColdSchema.
@@ -690,10 +699,14 @@ TEST(MemProf, YAMLParser) {
690699
AllocCount: 666
691700
TotalSize: 555
692701
CallSites:
693-
- - {Function: 0x500, LineOffset: 55, Column: 50, IsInlineFrame: true}
702+
- Frames:
703+
- {Function: 0x500, LineOffset: 55, Column: 50, IsInlineFrame: true}
694704
- {Function: 0x600, LineOffset: 66, Column: 60, IsInlineFrame: false}
695-
- - {Function: 0x700, LineOffset: 77, Column: 70, IsInlineFrame: true}
705+
CalleeGuids: [0x1000, 0x2000]
706+
- Frames:
707+
- {Function: 0x700, LineOffset: 77, Column: 70, IsInlineFrame: true}
696708
- {Function: 0x800, LineOffset: 88, Column: 80, IsInlineFrame: false}
709+
CalleeGuids: [0x3000]
697710
)YAML";
698711

699712
YAMLMemProfReader YAMLReader;
@@ -719,11 +732,19 @@ TEST(MemProf, YAMLParser) {
719732
ElementsAre(Frame(0x300, 33, 30, false), Frame(0x400, 44, 40, true)));
720733
EXPECT_EQ(Record.AllocSites[1].Info.getAllocCount(), 666U);
721734
EXPECT_EQ(Record.AllocSites[1].Info.getTotalSize(), 555U);
722-
EXPECT_THAT(Record.CallSites,
723-
ElementsAre(ElementsAre(Frame(0x500, 55, 50, true),
724-
Frame(0x600, 66, 60, false)),
725-
ElementsAre(Frame(0x700, 77, 70, true),
726-
Frame(0x800, 88, 80, false))));
735+
EXPECT_THAT(
736+
Record.CallSites,
737+
ElementsAre(
738+
AllOf(testing::Field(&CallSiteInfo::Frames,
739+
ElementsAre(Frame(0x500, 55, 50, true),
740+
Frame(0x600, 66, 60, false))),
741+
testing::Field(&CallSiteInfo::CalleeGuids,
742+
ElementsAre(0x1000, 0x2000))),
743+
AllOf(testing::Field(&CallSiteInfo::Frames,
744+
ElementsAre(Frame(0x700, 77, 70, true),
745+
Frame(0x800, 88, 80, false))),
746+
testing::Field(&CallSiteInfo::CalleeGuids,
747+
ElementsAre(0x3000)))));
727748
}
728749

729750
// Verify that the YAML parser accepts a GUID expressed as a function name.

0 commit comments

Comments
 (0)