Skip to content

Commit a3d2f0e

Browse files
author
Mike Kaufman
committed
[MERGE #5180 @mike-kaufman] updating RecyclerTelemetryInfo to use SList
Merge pull request #5180 from mike-kaufman:build/mkaufman/use-slist-for-gc-telemetry Changing Recycler telemetry to use SLIst instead of own simple linked-list. This has benefit of JD support.
2 parents 6c60ebf + 4c6bbff commit a3d2f0e

File tree

3 files changed

+90
-77
lines changed

3 files changed

+90
-77
lines changed

‎lib/Common/DataStructures/SList.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
//
1111
//----------------------------------------------------------------------------
1212

13+
#pragma once
14+
1315
class FakeCount
1416
{
1517
protected:
@@ -347,6 +349,19 @@ class SListBase : protected SListNodeBase<TAllocator>, public TCount
347349
return nullptr;
348350
}
349351

352+
Field(TData, TAllocator) * PrependNodeNoThrow(TAllocator * allocator)
353+
{
354+
Node * newNode = AllocatorNewNoThrow(TAllocator, allocator, Node);
355+
if (newNode)
356+
{
357+
newNode->Next() = this->Next();
358+
this->Next() = newNode;
359+
this->IncrementCount();
360+
return &newNode->data;
361+
}
362+
return nullptr;
363+
}
364+
350365
template <typename TParam>
351366
Field(TData, TAllocator) * PrependNode(TAllocator * allocator, TParam param)
352367
{

‎lib/Common/Memory/RecyclerTelemetryInfo.cpp

Lines changed: 69 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#ifdef ENABLE_BASIC_TELEMETRY
1010

1111
#include "Recycler.h"
12+
#include "DataStructures/SList.h"
1213
#include "HeapBucketStats.h"
1314
#include "BucketStatsReporter.h"
1415
#include <psapi.h>
@@ -24,7 +25,7 @@ namespace Memory
2425
RecyclerTelemetryInfo::RecyclerTelemetryInfo(Recycler * recycler, RecyclerTelemetryHostInterface* hostInterface) :
2526
passCount(0),
2627
hostInterface(hostInterface),
27-
lastPassStats(nullptr),
28+
gcPassStats(&HeapAllocator::Instance),
2829
recyclerStartTime(Js::Tick::Now()),
2930
abortTelemetryCapture(false),
3031
inPassActiveState(false),
@@ -35,11 +36,14 @@ namespace Memory
3536

3637
RecyclerTelemetryInfo::~RecyclerTelemetryInfo()
3738
{
38-
if (this->hostInterface != nullptr && this->passCount > 0)
39+
if (this->hostInterface != nullptr)
3940
{
4041
AssertOnValidThread(this, RecyclerTelemetryInfo::~RecyclerTelemetryInfo);
41-
this->hostInterface->TransmitTelemetry(*this);
42-
this->FreeGCPassStats();
42+
if (this->gcPassStats.Empty() == false)
43+
{
44+
this->hostInterface->TransmitTelemetry(*this);
45+
this->FreeGCPassStats();
46+
}
4347
}
4448
}
4549

@@ -70,32 +74,49 @@ namespace Memory
7074
sizes->numberOfSegments = allocator->GetNumberOfSegments();
7175
}
7276

77+
GCPassStatsList::Iterator RecyclerTelemetryInfo::GetGCPassStatsIterator() const
78+
{
79+
return this->gcPassStats.GetIterator();
80+
}
81+
82+
RecyclerTelemetryGCPassStats* RecyclerTelemetryInfo::GetLastPassStats() const
83+
{
84+
RecyclerTelemetryGCPassStats* stats = nullptr;
85+
if (this->gcPassStats.Empty() == false)
86+
{
87+
RecyclerTelemetryGCPassStats& ref = const_cast<RecyclerTelemetryGCPassStats&>(this->gcPassStats.Head());
88+
stats = &ref;
89+
}
90+
return stats;
91+
}
92+
7393
void RecyclerTelemetryInfo::StartPass()
7494
{
7595
this->inPassActiveState = false;
7696
if (this->ShouldStartTelemetryCapture())
7797
{
7898
Js::Tick start = Js::Tick::Now();
7999
AssertOnValidThread(this, RecyclerTelemetryInfo::StartPass);
100+
80101
#if DBG
81102
// validate state of existing GC pass stats structs
82103
uint16 count = 0;
83-
if (this->lastPassStats != nullptr)
104+
if (this->gcPassStats.Empty() == false)
84105
{
85-
RecyclerTelemetryGCPassStats* head = this->lastPassStats->next;
86-
RecyclerTelemetryGCPassStats* curr = head;
87-
do
106+
GCPassStatsList::Iterator it = this->GetGCPassStatsIterator();
107+
while (it.Next())
88108
{
89-
AssertMsg(curr->isGCPassActive == false, "unexpected value for isGCPassActive");
90-
count++;
91-
curr = curr->next;
92-
} while (curr != head);
109+
RecyclerTelemetryGCPassStats& curr = it.Data();
110+
AssertMsg(curr.isGCPassActive == false, "unexpected value for isGCPassActive");
111+
++count;
112+
}
93113
}
94114
AssertMsg(count == this->passCount, "RecyclerTelemetryInfo::StartPass() - mismatch between passCount and count.");
95115
#endif
96116

97-
RecyclerTelemetryGCPassStats* p = HeapNewNoThrow(RecyclerTelemetryGCPassStats);
98-
if (p == nullptr)
117+
118+
RecyclerTelemetryGCPassStats* stats = this->gcPassStats.PrependNodeNoThrow(&HeapAllocator::Instance);
119+
if (stats == nullptr)
99120
{
100121
// failed to allocate memory - disable any further telemetry capture for this recycler
101122
// and free any existing GC stats we've accumulated
@@ -107,41 +128,30 @@ namespace Memory
107128
{
108129
this->inPassActiveState = true;
109130
passCount++;
110-
memset(p, 0, sizeof(RecyclerTelemetryGCPassStats));
111-
if (this->lastPassStats == nullptr)
112-
{
113-
p->next = p;
114-
}
115-
else
116-
{
117-
p->next = lastPassStats->next;
118-
this->lastPassStats->next = p;
119-
}
120-
this->lastPassStats = p;
131+
memset(stats, 0, sizeof(RecyclerTelemetryGCPassStats));
121132

122-
this->lastPassStats->isGCPassActive = true;
123-
this->lastPassStats->passStartTimeTick = Js::Tick::Now();
124-
GetSystemTimePreciseAsFileTime(&this->lastPassStats->passStartTimeFileTime);
133+
stats->isGCPassActive = true;
134+
stats->passStartTimeTick = Js::Tick::Now();
135+
GetSystemTimePreciseAsFileTime(&stats->passStartTimeFileTime);
125136
if (this->hostInterface != nullptr)
126137
{
127138
LPFILETIME ft = this->hostInterface->GetLastScriptExecutionEndTime();
128-
this->lastPassStats->lastScriptExecutionEndTime = *ft;
139+
stats->lastScriptExecutionEndTime = *ft;
129140
}
130141

131-
this->lastPassStats->processCommittedBytes_start = RecyclerTelemetryInfo::GetProcessCommittedBytes();
132-
this->lastPassStats->processAllocaterUsedBytes_start = PageAllocator::GetProcessUsedBytes();
133-
this->lastPassStats->isInScript = this->recycler->GetIsInScript();
134-
this->lastPassStats->isScriptActive = this->recycler->GetIsScriptActive();
142+
stats->processCommittedBytes_start = RecyclerTelemetryInfo::GetProcessCommittedBytes();
143+
stats->processAllocaterUsedBytes_start = PageAllocator::GetProcessUsedBytes();
144+
stats->isInScript = this->recycler->GetIsInScript();
145+
stats->isScriptActive = this->recycler->GetIsScriptActive();
135146

136-
this->FillInSizeData(this->recycler->GetHeapInfo()->GetRecyclerLeafPageAllocator(), &this->lastPassStats->threadPageAllocator_start);
137-
this->FillInSizeData(this->recycler->GetHeapInfo()->GetRecyclerPageAllocator(), &this->lastPassStats->recyclerLeafPageAllocator_start);
138-
this->FillInSizeData(this->recycler->GetHeapInfo()->GetRecyclerLargeBlockPageAllocator(), &this->lastPassStats->recyclerLargeBlockPageAllocator_start);
147+
this->FillInSizeData(this->recycler->GetHeapInfo()->GetRecyclerLeafPageAllocator(), &stats->threadPageAllocator_start);
148+
this->FillInSizeData(this->recycler->GetHeapInfo()->GetRecyclerPageAllocator(), &stats->recyclerLeafPageAllocator_start);
149+
this->FillInSizeData(this->recycler->GetHeapInfo()->GetRecyclerLargeBlockPageAllocator(), &stats->recyclerLargeBlockPageAllocator_start);
139150
#ifdef RECYCLER_WRITE_BARRIER_ALLOC_SEPARATE_PAGE
140-
this->FillInSizeData(this->recycler->GetHeapInfo()->GetRecyclerWithBarrierPageAllocator(), &this->lastPassStats->recyclerWithBarrierPageAllocator_start);
151+
this->FillInSizeData(this->recycler->GetHeapInfo()->GetRecyclerWithBarrierPageAllocator(), &stats->recyclerWithBarrierPageAllocator_start);
141152
#endif
142-
this->lastPassStats->startPassProcessingElapsedTime = Js::Tick::Now() - start;
153+
stats->startPassProcessingElapsedTime = Js::Tick::Now() - start;
143154
}
144-
145155
}
146156
}
147157

@@ -156,34 +166,35 @@ namespace Memory
156166
Js::Tick start = Js::Tick::Now();
157167

158168
AssertOnValidThread(this, RecyclerTelemetryInfo::EndPass);
169+
RecyclerTelemetryGCPassStats* lastPassStats = this->GetLastPassStats();
159170

160-
this->lastPassStats->isGCPassActive = false;
161-
this->lastPassStats->passEndTimeTick = Js::Tick::Now();
171+
lastPassStats->isGCPassActive = false;
172+
lastPassStats->passEndTimeTick = Js::Tick::Now();
162173

163-
this->lastPassStats->processCommittedBytes_end = RecyclerTelemetryInfo::GetProcessCommittedBytes();
164-
this->lastPassStats->processAllocaterUsedBytes_end = PageAllocator::GetProcessUsedBytes();
174+
lastPassStats->processCommittedBytes_end = RecyclerTelemetryInfo::GetProcessCommittedBytes();
175+
lastPassStats->processAllocaterUsedBytes_end = PageAllocator::GetProcessUsedBytes();
165176

166-
this->FillInSizeData(this->recycler->GetHeapInfo()->GetRecyclerLeafPageAllocator(), &this->lastPassStats->threadPageAllocator_end);
167-
this->FillInSizeData(this->recycler->GetHeapInfo()->GetRecyclerPageAllocator(), &this->lastPassStats->recyclerLeafPageAllocator_end);
168-
this->FillInSizeData(this->recycler->GetHeapInfo()->GetRecyclerLargeBlockPageAllocator(), &this->lastPassStats->recyclerLargeBlockPageAllocator_end);
177+
this->FillInSizeData(this->recycler->GetHeapInfo()->GetRecyclerLeafPageAllocator(), &lastPassStats->threadPageAllocator_end);
178+
this->FillInSizeData(this->recycler->GetHeapInfo()->GetRecyclerPageAllocator(), &lastPassStats->recyclerLeafPageAllocator_end);
179+
this->FillInSizeData(this->recycler->GetHeapInfo()->GetRecyclerLargeBlockPageAllocator(), &lastPassStats->recyclerLargeBlockPageAllocator_end);
169180
#ifdef RECYCLER_WRITE_BARRIER_ALLOC_SEPARATE_PAGE
170-
this->FillInSizeData(this->recycler->GetHeapInfo()->GetRecyclerWithBarrierPageAllocator(), &this->lastPassStats->recyclerWithBarrierPageAllocator_end);
181+
this->FillInSizeData(this->recycler->GetHeapInfo()->GetRecyclerWithBarrierPageAllocator(), &lastPassStats->recyclerWithBarrierPageAllocator_end);
171182
#endif
172183

173184
// get bucket stats
174185
Js::Tick bucketStatsStart = Js::Tick::Now();
175186
BucketStatsReporter bucketReporter(this->recycler);
176187
this->recycler->GetHeapInfo()->GetBucketStats(bucketReporter);
177-
memcpy(&this->lastPassStats->bucketStats, bucketReporter.GetTotalStats(), sizeof(HeapBucketStats));
178-
this->lastPassStats->computeBucketStatsElapsedTime = Js::Tick::Now() - bucketStatsStart;
188+
memcpy(&lastPassStats->bucketStats, bucketReporter.GetTotalStats(), sizeof(HeapBucketStats));
189+
lastPassStats->computeBucketStatsElapsedTime = Js::Tick::Now() - bucketStatsStart;
179190

180-
this->lastPassStats->endPassProcessingElapsedTime = Js::Tick::Now() - start;
191+
lastPassStats->endPassProcessingElapsedTime = Js::Tick::Now() - start;
181192

182193
if (ShouldTransmit() && this->hostInterface != nullptr)
183194
{
184195
if (this->hostInterface->TransmitTelemetry(*this))
185196
{
186-
this->lastTransmitTime = this->lastPassStats->passEndTimeTick;
197+
this->lastTransmitTime = lastPassStats->passEndTimeTick;
187198
Reset();
188199
}
189200
}
@@ -200,27 +211,11 @@ namespace Memory
200211

201212
void RecyclerTelemetryInfo::FreeGCPassStats()
202213
{
203-
if (this->lastPassStats != nullptr)
214+
if (this->gcPassStats.Empty() == false)
204215
{
216+
AssertMsg(this->passCount > 0, "unexpected value for passCount");
205217
AssertOnValidThread(this, RecyclerTelemetryInfo::FreeGCPassStats);
206-
RecyclerTelemetryGCPassStats* head = this->lastPassStats->next;
207-
RecyclerTelemetryGCPassStats* curr = head;
208-
#ifdef DBG
209-
uint16 freeCount = 0;
210-
#endif
211-
do
212-
{
213-
RecyclerTelemetryGCPassStats* next = curr->next;
214-
HeapDelete(curr);
215-
curr = next;
216-
#ifdef DBG
217-
freeCount++;
218-
#endif
219-
} while (curr != head);
220-
#ifdef DBG
221-
AssertMsg(freeCount == this->passCount, "Unexpected mismatch between freeCount and passCount");
222-
#endif
223-
this->lastPassStats = nullptr;
218+
this->gcPassStats.Clear();
224219
this->passCount = 0;
225220
}
226221
}
@@ -233,17 +228,18 @@ namespace Memory
233228

234229
void RecyclerTelemetryInfo::IncrementUserThreadBlockedCount(Js::TickDelta waitTime, RecyclerWaitReason caller)
235230
{
231+
RecyclerTelemetryGCPassStats* lastPassStats = this->GetLastPassStats();
236232
#ifdef DBG
237233
if (this->inPassActiveState)
238234
{
239-
AssertMsg(this->lastPassStats != nullptr && this->lastPassStats->isGCPassActive == true, "unexpected Value in RecyclerTelemetryInfo::IncrementUserThreadBlockedCount");
235+
AssertMsg(lastPassStats != nullptr && lastPassStats->isGCPassActive == true, "unexpected Value in RecyclerTelemetryInfo::IncrementUserThreadBlockedCount");
240236
}
241237
#endif
242238

243-
if (this->inPassActiveState && this->lastPassStats != nullptr)
239+
if (this->inPassActiveState && lastPassStats != nullptr)
244240
{
245241
AssertOnValidThread(this, RecyclerTelemetryInfo::IncrementUserThreadBlockedCount);
246-
this->lastPassStats->uiThreadBlockedTimes[caller] += waitTime;
242+
lastPassStats->uiThreadBlockedTimes[caller] += waitTime;
247243
}
248244
}
249245

‎lib/Common/Memory/RecyclerTelemetryInfo.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "Common/Tick.h"
1111
#include "AllocatorTelemetryStats.h"
1212
#include "HeapBucketStats.h"
13+
#include "DataStructures/SList.h"
1314

1415
#ifdef ENABLE_BASIC_TELEMETRY
1516
#include "Windows.h"
@@ -50,7 +51,6 @@ namespace Memory
5051
Js::TickDelta endPassProcessingElapsedTime;
5152
Js::TickDelta computeBucketStatsElapsedTime;
5253
FILETIME lastScriptExecutionEndTime;
53-
RecyclerTelemetryGCPassStats* next;
5454
Js::TickDelta uiThreadBlockedTimes[static_cast<size_t>(RecyclerWaitReason::Other) + 1];
5555
bool isInScript;
5656
bool isScriptActive;
@@ -76,6 +76,8 @@ namespace Memory
7676
#endif
7777
};
7878

79+
typedef SList<RecyclerTelemetryGCPassStats, HeapAllocator> GCPassStatsList;
80+
7981
/**
8082
*
8183
*/
@@ -90,12 +92,13 @@ namespace Memory
9092
void IncrementUserThreadBlockedCount(Js::TickDelta waitTime, RecyclerWaitReason source);
9193

9294
inline const Js::Tick& GetRecyclerStartTime() const { return this->recyclerStartTime; }
93-
inline const RecyclerTelemetryGCPassStats* GetLastPassStats() const { return this->lastPassStats; }
95+
RecyclerTelemetryGCPassStats* GetLastPassStats() const;
9496
inline const Js::Tick& GetLastTransmitTime() const { return this->lastTransmitTime; }
9597
inline const uint16 GetPassCount() const { return this->passCount; }
9698
const GUID& GetRecyclerID() const;
9799
bool GetIsConcurrentEnabled() const;
98100
bool IsOnScriptThread() const;
101+
GCPassStatsList::Iterator GetGCPassStatsIterator() const;
99102

100103
AllocatorDecommitStats* GetThreadPageAllocator_decommitStats() { return &this->threadPageAllocator_decommitStats; }
101104
AllocatorDecommitStats* GetRecyclerLeafPageAllocator_decommitStats() { return &this->recyclerLeafPageAllocator_decommitStats; }
@@ -111,8 +114,7 @@ namespace Memory
111114
Js::Tick recyclerStartTime;
112115
bool inPassActiveState;
113116

114-
// TODO: update list below to SList. Need to ensure we have same allocation semantics (specifically heap allocs, no exceptions on failure)
115-
RecyclerTelemetryGCPassStats* lastPassStats;
117+
GCPassStatsList gcPassStats;
116118
Js::Tick lastTransmitTime;
117119
uint16 passCount;
118120
bool abortTelemetryCapture;

0 commit comments

Comments
 (0)