[EventPipe][UserEvents] Deduplicate EventPipeEvent Metadata emission

src/coreclr/nativeaot/Runtime/eventpipe/ep-rt-aot.cpp

@@ -331,6 +331,12 @@ ep_rt_aot_atomic_dec_int64_t (volatile int64_t *value) {
     return (currentValue - 1);
 }
 
+int64_t
+ep_rt_aot_atomic_compare_exchange_int64_t (volatile int64_t *target, int64_t expected, int64_t value) {
+    STATIC_CONTRACT_NOTHROW;
+    return static_cast<int64_t>(PalInterlockedCompareExchange64 ((volatile int64_t *)target, (int64_t)value, (int64_t)expected));
+}
+
 size_t
 ep_rt_aot_atomic_compare_exchange_size_t (volatile size_t *target, size_t expected, size_t value) {
     STATIC_CONTRACT_NOTHROW;

src/coreclr/nativeaot/Runtime/eventpipe/ep-rt-aot.h

@@ -224,6 +224,16 @@ ep_rt_atomic_dec_int64_t (volatile int64_t *value)
     return ep_rt_aot_atomic_dec_int64_t (value);
 }
 
+static
+inline
+int64_t
+ep_rt_atomic_compare_exchange_int64_t (volatile int64_t *target, int64_t expected, int64_t value)
+{
+    STATIC_CONTRACT_NOTHROW;
+    extern int64_t ep_rt_aot_atomic_compare_exchange_int64_t (volatile int64_t *target, int64_t expected, int64_t value);
+    return ep_rt_aot_atomic_compare_exchange_int64_t (target, expected, value);
+}
+
 static
 inline
 size_t

src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.h

@@ -229,6 +229,15 @@ ep_rt_atomic_dec_int64_t (volatile int64_t *value)
 	return static_cast<int64_t>(InterlockedDecrement64 ((volatile LONG64 *)(value)));
 }
 
+static
+inline
+int64_t
+ep_rt_atomic_compare_exchange_int64_t (volatile int64_t *target, int64_t expected, int64_t value)
+{
+	STATIC_CONTRACT_NOTHROW;
+	return static_cast<int64_t>(InterlockedCompareExchangeT<int64_t> (target, value, expected));
+}
+
 static
 inline
 size_t

src/mono/mono/eventpipe/ep-rt-mono.h

@@ -355,6 +355,14 @@ ep_rt_atomic_dec_int64_t (volatile int64_t *value)
 	return (int64_t)mono_atomic_dec_i64 ((volatile gint64 *)value);
 }
 
+static
+inline
+int64_t
+ep_rt_atomic_compare_exchange_int64_t (volatile int64_t *target, int64_t expected, int64_t value)
+{
+	return (int64_t)(mono_atomic_cas_i64 ((volatile gint64 *)(target), (gint64)(value), (gint64)(expected)));
+}
+
 static
 inline
 size_t

src/native/eventpipe/ep-event.c

@@ -72,6 +72,7 @@ ep_event_alloc (
 	instance->level = level;
 	instance->need_stack = need_stack;
 	instance->enabled_mask = 0;
+	instance->metadata_written_mask = 0;
 
 	if (metadata != NULL) {
 		instance->metadata = ep_rt_byte_array_alloc (metadata_len);
@@ -102,6 +103,30 @@ ep_event_free (EventPipeEvent *ep_event)
 	ep_rt_object_free (ep_event);
 }
 
+bool
+ep_event_update_metadata_written_mask (
+	EventPipeEvent *ep_event,
+	uint64_t session_mask,
+	bool enable)
+{
+	EP_ASSERT (ep_event != NULL);
+	EP_ASSERT (session_mask != 0);
+
+	int64_t expected_value;
+	int64_t previous_value;
+
+	do {
+		expected_value = ep_rt_volatile_load_int64_t (&ep_event->metadata_written_mask);
+		if (((expected_value & session_mask) != 0) == enable)
+			return false; // Already set to the desired state.
+
+		int64_t new_value = enable ? (expected_value | session_mask) : (expected_value & ~session_mask);
+		previous_value = ep_rt_atomic_compare_exchange_int64_t (&ep_event->metadata_written_mask, expected_value, new_value);
+	} while (previous_value != expected_value);
+
+	return true;
+}
+
 #endif /* !defined(EP_INCLUDE_SOURCE_FILES) || defined(EP_FORCE_INCLUDE_SOURCE_FILES) */
 #endif /* ENABLE_PERFTRACING */
 

src/native/eventpipe/ep-event.h

@@ -26,6 +26,9 @@ struct _EventPipeEvent_Internal {
 	uint64_t keywords;
 	// The ith bit is 1 iff the event is enabled for the ith session.
 	volatile int64_t enabled_mask;
+	// The ith bit is 1 iff the event metadata has been written for the ith session.
+	// Currently, bits are only enabled in user_events sessions.
+	volatile int64_t metadata_written_mask;
 	// Metadata
 	uint8_t *metadata;
 	// The provider that contains the event.
@@ -92,5 +95,11 @@ ep_event_alloc (
 void
 ep_event_free (EventPipeEvent * ep_event);
 
+bool
+ep_event_update_metadata_written_mask (
+	EventPipeEvent *ep_event,
+	uint64_t session_mask,
+	bool enable);
+
 #endif /* ENABLE_PERFTRACING */
 #endif /* __EVENTPIPE_EVENT_H__ */

src/native/eventpipe/ep-provider.c

@@ -35,12 +35,16 @@ provider_prepare_callback_data (
 // _Requires_lock_held (ep)
 static
 void
-provider_refresh_all_events (EventPipeProvider *provider);
+provider_refresh_all_events (
+	EventPipeProvider *provider,
+	uint64_t session_mask);
 
 // _Requires_lock_held (ep)
 static
 void
-provider_refresh_event_state (EventPipeEvent *ep_event);
+provider_refresh_event_state (
+	EventPipeEvent *ep_event,
+	uint64_t session_mask);
 
 // Compute the enabled bit mask, the ith bit is 1 iff an event with the
 // given (provider, keywords, eventLevel) is enabled for the ith session.
@@ -96,7 +100,9 @@ provider_prepare_callback_data (
 
 static
 void
-provider_refresh_all_events (EventPipeProvider *provider)
+provider_refresh_all_events (
+	EventPipeProvider *provider,
+	uint64_t session_mask)
 {
 	EP_ASSERT (provider != NULL);
 
@@ -105,7 +111,7 @@ provider_refresh_all_events (EventPipeProvider *provider)
 	EP_ASSERT (provider->event_list != NULL);
 
 	DN_LIST_FOREACH_BEGIN (EventPipeEvent *, current_event, provider->event_list) {
-		provider_refresh_event_state (current_event);
+		provider_refresh_event_state (current_event, session_mask);
 	} DN_LIST_FOREACH_END;
 
 	ep_requires_lock_held ();
@@ -114,7 +120,9 @@ provider_refresh_all_events (EventPipeProvider *provider)
 
 static
 void
-provider_refresh_event_state (EventPipeEvent *ep_event)
+provider_refresh_event_state (
+	EventPipeEvent *ep_event,
+	uint64_t session_mask)
 {
 	EP_ASSERT (ep_event != NULL);
 
@@ -129,6 +137,11 @@ provider_refresh_event_state (EventPipeEvent *ep_event)
 	int64_t enable_mask = provider_compute_event_enable_mask (config, provider, ep_event);
 	ep_event_set_enabled_mask (ep_event, enable_mask);
 
+	// If session_mask is not zero, that session is being enabled/disabled.
+	// We need to unset the metadata_written_mask for this session.
+	if (session_mask != 0)
+		ep_event_update_metadata_written_mask (ep_event, session_mask, false);
+
 	ep_requires_lock_held ();
 	return;
 }
@@ -277,7 +290,7 @@ ep_provider_add_event (
 	// Take the config lock before inserting a new event.
 	EP_LOCK_ENTER (section1)
 		ep_raise_error_if_nok_holding_lock (dn_list_push_back (provider->event_list, instance), section1);
-		provider_refresh_event_state (instance);
+		provider_refresh_event_state (instance, 0);
 	EP_LOCK_EXIT (section1)
 
 ep_on_exit:
@@ -323,7 +336,7 @@ provider_set_config (
 	provider->keywords = keywords_for_all_sessions;
 	provider->provider_level = level_for_all_sessions;
 
-	provider_refresh_all_events (provider);
+	provider_refresh_all_events (provider, session_mask);
 	provider_prepare_callback_data (provider, provider->keywords, provider->provider_level, filter_data, callback_data, session_id);
 
 	ep_requires_lock_held ();
@@ -352,7 +365,7 @@ provider_unset_config (
 	provider->keywords = keywords_for_all_sessions;
 	provider->provider_level = level_for_all_sessions;
 
-	provider_refresh_all_events (provider);
+	provider_refresh_all_events (provider, session_mask);
 	provider_prepare_callback_data (provider, provider->keywords, provider->provider_level, filter_data, callback_data, (EventPipeSessionID)0);
 
 	ep_requires_lock_held ();
@@ -517,7 +530,7 @@ provider_add_event (
 	ep_raise_error_if_nok (instance != NULL);
 
 	ep_raise_error_if_nok (dn_list_push_back (provider->event_list, instance));
-	provider_refresh_event_state (instance);
+	provider_refresh_event_state (instance, 0);
 
 ep_on_exit:
 	ep_requires_lock_held ();

src/native/eventpipe/ep-rt.h

@@ -79,6 +79,10 @@ static
 int64_t
 ep_rt_atomic_dec_int64_t (volatile int64_t *value);
 
+static
+int64_t
+ep_rt_atomic_compare_exchange_int64_t (volatile int64_t *target, int64_t expected, int64_t value);
+
 static
 size_t
 ep_rt_atomic_compare_exchange_size_t (volatile size_t *target, size_t expected, size_t value);

src/native/eventpipe/ep-session.c

@@ -815,20 +815,29 @@ session_tracepoint_write_event (
 	// Extension
 	uint32_t extension_len = 0;
 
-	// Extension Event Metadata
-	uint32_t metadata_len = ep_event_get_metadata_len (ep_event);
-	uint8_t extension_metadata[1 + sizeof(uint32_t)];
-	extension_metadata[0] = 0x01; // label
-	*(uint32_t*)&extension_metadata[1] = metadata_len;
-	io[io_index].iov_base = extension_metadata;
-	io[io_index].iov_len = sizeof(extension_metadata);
-	io_index++;
-	extension_len += sizeof(extension_metadata);
+	// We make a best-effort to write out metadata on the first instance of each event in
+	// the trace. Even though the atomic InterlockedExchange will detect the first event
+	// writing thread to race to this point there is no guarantee that same thread will
+	// be the first one to invoke writev and place its data into the kernel buffers. This
+	// means it is still possible that the metadata isn't on the first event in the
+	// trace. If the trace ended fast enough there is even a tiny chance that it stops
+	// recording events before the event with the metadata ever gets recorded.
+	bool should_write_metadata = ep_event_update_metadata_written_mask (ep_event, ep_session_get_mask (session), true);
+	if (should_write_metadata) {
+		uint8_t extension_metadata[1 + sizeof(uint32_t)];
+		uint32_t metadata_len = ep_event_get_metadata_len (ep_event);
+		extension_metadata[0] = 0x01; // label
+		*(uint32_t*)&extension_metadata[1] = metadata_len;
+		io[io_index].iov_base = extension_metadata;
+		io[io_index].iov_len = sizeof(extension_metadata);
+		io_index++;
+		extension_len += sizeof(extension_metadata);
 
-	io[io_index].iov_base = (void *)ep_event_get_metadata (ep_event);
-	io[io_index].iov_len = metadata_len;
-	io_index++;
-	extension_len += metadata_len;
+		io[io_index].iov_base = (void *)ep_event_get_metadata (ep_event);
+		io[io_index].iov_len = metadata_len;
+		io_index++;
+		extension_len += metadata_len;
+	}
 
 	// Extension Activity IDs
 	const int extension_activity_ids_max_len = 2 * (1 + EP_ACTIVITY_ID_SIZE);