[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.
@@ -51,6 +54,7 @@ struct _EventPipeEvent {
 EP_DEFINE_GETTER(EventPipeEvent *, event, uint64_t, keywords)
 EP_DEFINE_GETTER_REF(EventPipeEvent *, event, volatile int64_t *, enabled_mask)
 EP_DEFINE_SETTER(EventPipeEvent *, event, int64_t, enabled_mask)
+EP_DEFINE_GETTER_REF(EventPipeEvent *, event, volatile int64_t *, metadata_written_mask)
 EP_DEFINE_GETTER(EventPipeEvent *, event, uint8_t *, metadata)
 EP_DEFINE_GETTER(EventPipeEvent *, event, EventPipeProvider *, provider)
 EP_DEFINE_GETTER(EventPipeEvent *, event, uint32_t, event_id)
@@ -92,5 +96,22 @@ ep_event_alloc (
 void
 ep_event_free (EventPipeEvent * ep_event);
 
+static
+inline
+bool
+ep_event_was_metadata_written (
+	const EventPipeEvent *ep_event,
+	uint64_t session_mask)
+{
+	EP_ASSERT (ep_event != NULL);
+	return ((ep_rt_volatile_load_int64_t (ep_event_get_metadata_written_mask_cref (ep_event)) & session_mask) != 0);
+}
+
+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

@@ -769,6 +769,7 @@ session_tracepoint_write_event (
 	const int max_static_io_capacity = 30; // Should account for most events that use EventData structs
 	struct iovec static_io[max_static_io_capacity];
 	struct iovec *io = static_io;
+	ssize_t io_bytes_to_write = 0;
 
 	uint8_t *ep_event_data = ep_event_payload_get_data (ep_event_payload);
 	uint32_t ep_event_data_len = ep_event_payload_get_event_data_len (ep_event_payload);
@@ -787,48 +788,58 @@ session_tracepoint_write_event (
 	io[io_index].iov_base = (void *)&write_index;
 	io[io_index].iov_len = sizeof(write_index);
 	io_index++;
+	io_bytes_to_write += sizeof(write_index);
 
 	// Version
 	uint8_t version = 0x01; // Format V1
 	io[io_index].iov_base = &version;
 	io[io_index].iov_len = sizeof(version);
 	io_index++;
+	io_bytes_to_write += sizeof(version);
 
 	// Truncated event id
 	// For parity with EventSource, there shouldn't be any that need more than 16 bits.
 	uint16_t truncated_event_id = ep_event_get_event_id (ep_event) & 0xFFFF;
 	io[io_index].iov_base = &truncated_event_id;
 	io[io_index].iov_len = sizeof(truncated_event_id);
 	io_index++;
+	io_bytes_to_write += sizeof(truncated_event_id);
 
 	// Extension and payload relative locations (to be fixed up later)
 	uint32_t extension_rel_loc = 0;
 	io[io_index].iov_base = &extension_rel_loc;
 	io[io_index].iov_len = sizeof(extension_rel_loc);
 	io_index++;
+	io_bytes_to_write += sizeof(extension_rel_loc);
 
 	uint32_t payload_rel_loc = 0;
 	io[io_index].iov_base = &payload_rel_loc;
 	io[io_index].iov_len = sizeof(payload_rel_loc);
 	io_index++;
+	io_bytes_to_write += sizeof(payload_rel_loc);
 
 	// 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);
+	uint64_t session_mask = ep_session_get_mask (session);
+	bool should_write_metadata = !ep_event_was_metadata_written (ep_event, session_mask);
+	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_bytes_to_write += 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;
+		io_bytes_to_write += metadata_len;
+	}
 
 	// Extension Activity IDs
 	const int extension_activity_ids_max_len = 2 * (1 + EP_ACTIVITY_ID_SIZE);
@@ -839,6 +850,7 @@ session_tracepoint_write_event (
 	io[io_index].iov_len = extension_activity_ids_len;
 	io_index++;
 	extension_len += extension_activity_ids_len;
+	io_bytes_to_write += extension_activity_ids_len;
 
 	EP_ASSERT (io_index <= max_non_parameter_iov);
 
@@ -850,6 +862,7 @@ session_tracepoint_write_event (
 		io[io_index].iov_len = ep_event_payload_size;
 		io_index++;
 		ep_event_payload_total_size += ep_event_payload_size;
+		io_bytes_to_write += ep_event_payload_size;
 	} else {
 		const EventData *event_data = ep_event_payload_get_event_data (ep_event_payload);
 		for (uint32_t i = 0; i < ep_event_data_len; ++i) {
@@ -858,6 +871,7 @@ session_tracepoint_write_event (
 			io[io_index].iov_len = ep_event_data_size;
 			io_index++;
 			ep_event_payload_total_size += ep_event_data_size;
+			io_bytes_to_write += ep_event_data_size;
 		}
 	}
 
@@ -872,7 +886,9 @@ session_tracepoint_write_event (
 	payload_rel_loc = ep_event_payload_total_size << 16 | (extension_len & 0xFFFF);
 
 	ssize_t bytes_written;
-	while ((bytes_written = writev(session->user_events_data_fd, (const struct iovec *)io, io_index) < 0) && errno == EINTR);
+	while (((bytes_written = writev(session->user_events_data_fd, (const struct iovec *)io, io_index)) < 0) && errno == EINTR);
+	if (should_write_metadata && (bytes_written == io_bytes_to_write))
+		ep_event_update_metadata_written_mask (ep_event, session_mask, true);
 
 	if (io != static_io)
 		free (io);