Skip to content

Commit 86b0fad

Browse files
committed
[EventPipe] Restrict SequencePoint sequence numbers
Previously, EventPipeThread's lifetimes were inherently tied to the lifetime of all SequencePoints that track the thread's last sequence number. In adding ThreadSessionState cleanup, SequencePoints forced deletions to be deferred. SequencePoints do not need to extend EventPipeThreads lifetimes, as they only need to track the last known sequence number of that thread by the time the SequencePoint is written. To disassociate the two lifetimes and allow ThreadSessionStates to be cleaned up as soon as possible: 1) Update SequencePoints in tandem with event processing Delaying SequencePoint updating until the SequencePoint is written requires the buffer_manager's thread_session_state_list to not trim any ThreadSessionStates. Updating the mapped ThreadId:sequence_number in tandem allows the thread_session_state_list to be trimmed immediately. 2) Restrict ThreadId:sequence_number tracking Previously, SequencePoints tracked all threads and their last known sequence number regardless of whether there have been new events, and mandated reactionary removal when the ThreadSessionState was being deleted. Instead, by tightening the conditions for when a SequencePoint should track a particulr thread, they should never need to track threads unnecessarily. The conditions for when a SequencePoint needs to track a thread are: a) EventPipeThread is registered during SequencePoint init or b) EventPipeThread is unregistered during SP init and there are new event write attempts since the last SP
1 parent 11feac8 commit 86b0fad

File tree

4 files changed

+68
-0
lines changed

4 files changed

+68
-0
lines changed

src/native/eventpipe/ep-buffer-manager.c

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,10 @@ buffer_manager_init_sequence_point_thread_list (
399399
ep_buffer_manager_requires_lock_held (buffer_manager);
400400

401401
DN_LIST_FOREACH_BEGIN (EventPipeThreadSessionState *, thread_session_state, buffer_manager->thread_session_state_list) {
402+
EventPipeThreadSequencePointTrackState prev_track_state = ep_thread_session_state_get_track_state (thread_session_state);
403+
if (prev_track_state == EP_SEQUENCE_POINT_THREAD_ID_RETIRED)
404+
continue;
405+
402406
// The sequence number captured here is not guaranteed to be the most recent sequence number, nor
403407
// is it guaranteed to match the number of events we would observe in the thread's write buffer
404408
// memory. This is only used as a lower bound on the number of events the thread has attempted to
@@ -408,6 +412,22 @@ buffer_manager_init_sequence_point_thread_list (
408412
// event is one less. Sequence numbers are allowed to overflow, so going backwards is allowed to
409413
// underflow.
410414
uint32_t sequence_number = ep_thread_session_state_get_volatile_sequence_number (thread_session_state) - 1;
415+
if (ep_rt_volatile_load_uint32_t_without_barrier (ep_thread_get_unregistered_ref (ep_thread_session_state_get_thread (thread_session_state))) > 0) {
416+
// This sequence point will be the last one that embeds this thread's ID as there will be no more
417+
// writes by this thread and thus all its events are prior to this sequence point.
418+
// Future sequence points do not need to track this thread's ID
419+
ep_thread_session_state_set_track_state (thread_session_state, EP_SEQUENCE_POINT_THREAD_ID_RETIRED);
420+
421+
if ((ep_thread_session_state_get_last_read_sequence_number (thread_session_state) == sequence_number) &&
422+
(prev_track_state == EP_SEQUENCE_POINT_THREAD_ID_TRACKED)) {
423+
// The thread will not emit new events, and the last sequence point has already tracked
424+
// the last sequence number emitted by this thread,
425+
// so we don't need to track it anymore
426+
continue;
427+
}
428+
} else if (prev_track_state == EP_SEQUENCE_POINT_THREAD_ID_UNTRACKED) {
429+
ep_thread_session_state_set_track_state (thread_session_state, EP_SEQUENCE_POINT_THREAD_ID_TRACKED);
430+
}
411431

412432
dn_umap_ptr_uint32_insert (ep_sequence_point_get_thread_sequence_numbers (sequence_point), thread_session_state, sequence_number);
413433
ep_thread_addref (ep_thread_holder_get_thread (ep_thread_session_state_get_thread_holder_ref (thread_session_state)));
@@ -1189,6 +1209,12 @@ ep_buffer_manager_write_all_buffers_to_file_v4 (
11891209
}
11901210

11911211
ep_thread_session_state_set_last_read_sequence_number (current_thread_session_state, sequence_number);
1212+
if (sequence_point != NULL) {
1213+
dn_umap_it_t found = dn_umap_ptr_uint32_find (ep_sequence_point_get_thread_sequence_numbers (sequence_point), current_thread_session_state);
1214+
EP_ASSERT (!dn_umap_it_end (found));
1215+
dn_umap_ptr_uint32_insert_or_assign (ep_sequence_point_get_thread_sequence_numbers (sequence_point), current_thread_session_state, sequence_number);
1216+
}
1217+
11921218
// Have we written events in any sequence point?
11931219
*events_written = events_written_for_thread || *events_written;
11941220
}
@@ -1211,6 +1237,7 @@ ep_buffer_manager_write_all_buffers_to_file_v4 (
12111237
dn_umap_it_t found = dn_umap_ptr_uint32_find (ep_sequence_point_get_thread_sequence_numbers (sequence_point), session_state);
12121238
uint32_t thread_sequence_number = !dn_umap_it_end (found) ? dn_umap_it_value_uint32_t (found) : 0;
12131239
uint32_t last_read_sequence_number = ep_thread_session_state_get_last_read_sequence_number (session_state);
1240+
EP_ASSERT (thread_sequence_number == 0 || thread_sequence_number == last_read_sequence_number);
12141241
// Sequence numbers can overflow so we can't use a direct last_read > sequence_number comparison
12151242
// If a thread is able to drop more than 0x80000000 events in between sequence points then we will
12161243
// miscategorize it, but that seems unlikely.
@@ -1238,6 +1265,7 @@ ep_buffer_manager_write_all_buffers_to_file_v4 (
12381265
buffer_manager_remove_and_delete_thread_session_state (buffer_manager, thread_session_state);
12391266
} DN_VECTOR_PTR_FOREACH_END;
12401267
buffer_manager_dequeue_sequence_point (buffer_manager);
1268+
sequence_point = NULL;
12411269
EP_SPIN_LOCK_EXIT (&buffer_manager->rt_lock, section3)
12421270
}
12431271

src/native/eventpipe/ep-thread.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,7 @@ ep_thread_session_state_alloc (
339339
ep_raise_error_if_nok (instance->buffer_list != NULL);
340340

341341
instance->delete_deferred = false;
342+
instance->track_state = EP_SEQUENCE_POINT_THREAD_ID_UNTRACKED;
342343

343344
ep_on_exit:
344345
return instance;

src/native/eventpipe/ep-thread.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,7 @@ struct _EventPipeThreadSessionState_Internal {
302302
// buffer_manager_advance_to_non_empty_buffer, but sequence points are active,
303303
// so we defer the deletion until the next sequence point updates its map.
304304
bool delete_deferred;
305+
EventPipeThreadSequencePointTrackState track_state;
305306
};
306307

307308
#if !defined(EP_INLINE_GETTER_SETTER) && !defined(EP_IMPL_THREAD_GETTER_SETTER)
@@ -316,6 +317,8 @@ EP_DEFINE_GETTER(EventPipeThreadSessionState *, thread_session_state, uint32_t,
316317
EP_DEFINE_SETTER(EventPipeThreadSessionState *, thread_session_state, uint32_t, last_read_sequence_number)
317318
EP_DEFINE_GETTER(EventPipeThreadSessionState *, thread_session_state, bool, delete_deferred)
318319
EP_DEFINE_SETTER(EventPipeThreadSessionState *, thread_session_state, bool, delete_deferred)
320+
EP_DEFINE_GETTER(EventPipeThreadSessionState *, thread_session_state, EventPipeThreadSequencePointTrackState, track_state)
321+
EP_DEFINE_SETTER(EventPipeThreadSessionState *, thread_session_state, EventPipeThreadSequencePointTrackState, track_state)
319322

320323
EventPipeThreadSessionState *
321324
ep_thread_session_state_alloc (

src/native/eventpipe/ep-types-forward.h

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,42 @@ typedef enum {
169169
EP_THREAD_TYPE_SAMPLING
170170
} EventPipeThreadType;
171171

172+
/**
173+
* Tracks the lifecycle of a ThreadSessionState in relation to SequencePoint tracking.
174+
*
175+
* SequencePoints capture a snapshot of thread sequence numbers at a point in time to establish
176+
* ordering boundaries for event processing. This enum tracks whether a thread's sequence number
177+
* needs to be included in SequencePoint snapshots.
178+
*
179+
* State transitions:
180+
* UNTRACKED -> TRACKED: When a thread first participates in event writing after a SequencePoint is initialized
181+
* TRACKED -> RETIRED: When a thread unregisters and will write no more events
182+
*
183+
* The UNTRACKED state exists to handle threads that write events and unregister before a SequencePoint
184+
* is initialized. In this scenario, the thread's last known sequence number still needs to be tracked
185+
* by the SequencePoint to maintain proper event ordering boundaries. However, if a thread has no new
186+
* event writes between two SequencePoints and unregisters before the second SequencePoint, that
187+
* SequencePoint does not need to track it.
188+
*
189+
* UNTRACKED (0): The thread has not yet been tracked by any SequencePoint since the last SequencePoint
190+
* was written. This is the initial state for new ThreadSessionStates, and also applies
191+
* to threads that wrote events but unregistered before a SequencePoint could track them.
192+
*
193+
* TRACKED (1): The thread is actively being tracked by SequencePoints. The thread's sequence number
194+
* will be captured in SequencePoint snapshots to establish event ordering boundaries.
195+
*
196+
* RETIRED (2): The thread has unregistered and will write no more events. Once a thread unregisters
197+
* before a SequencePoint, there will never be any events written by the thread past that SequencePoint,
198+
* and therefore no future SequencePoints will need to track that thread's sequence numbers. The final
199+
* sequence number has been captured, and future SequencePoints do not need to track this thread unless
200+
* there are still unread events from this thread.
201+
*/
202+
typedef enum {
203+
EP_SEQUENCE_POINT_THREAD_ID_UNTRACKED = 0,
204+
EP_SEQUENCE_POINT_THREAD_ID_TRACKED = 1,
205+
EP_SEQUENCE_POINT_THREAD_ID_RETIRED = 2
206+
} EventPipeThreadSequencePointTrackState;
207+
172208
/*
173209
* EventPipe Basic Types.
174210
*/

0 commit comments

Comments
 (0)