Merge pull request DataDog#4254 from DataDog/ivoanjo/use-typedef-instead-of-struct

[NO-TICKET] Profiling refactor: typedef most structures

Author: ivoanjo

ext/datadog_profiling_native_extension/clock_id.h

@@ -5,13 +5,13 @@
 #include <ruby.h>
 
 // Contains the operating-system specific identifier needed to fetch CPU-time, and a flag to indicate if we failed to fetch it
-typedef struct thread_cpu_time_id {
+typedef struct {
   bool valid;
   clockid_t clock_id;
 } thread_cpu_time_id;
 
 // Contains the current cpu time, and a flag to indicate if we failed to fetch it
-typedef struct thread_cpu_time {
+typedef struct {
   bool valid;
   long result_ns;
 } thread_cpu_time;

ext/datadog_profiling_native_extension/collectors_cpu_and_wall_time_worker.c

@@ -333,7 +333,7 @@ static VALUE _native_should_sample(VALUE self, VALUE now);
 static VALUE _native_after_sample(VALUE self, VALUE now);
 static VALUE _native_state_snapshot(VALUE self);
 
-typedef struct sampler_state {
+typedef struct {
   discrete_dynamic_sampler sampler;
 } sampler_state;
 

ext/datadog_profiling_native_extension/collectors_discrete_dynamic_sampler.c

@@ -16,7 +16,7 @@
 //       every event and is thus, in theory, susceptible to some pattern
 //       biases. In practice, the dynamic readjustment of sampling interval
 //       and randomized starting point should help with avoiding heavy biases.
-typedef struct discrete_dynamic_sampler {
+typedef struct {
   // --- Config ---
   // Name of this sampler for debug logs.
   const char *debug_name;

ext/datadog_profiling_native_extension/collectors_discrete_dynamic_sampler.h

@@ -21,15 +21,15 @@
 typedef enum { ACTION_WAIT, ACTION_RUN, ACTION_STOP } action;
 
 // Contains state for a single CpuAndWallTimeWorker instance
-struct idle_sampling_loop_state {
+typedef struct {
   pthread_mutex_t wakeup_mutex;
   pthread_cond_t wakeup;
   action requested_action;
   void (*run_action_function)(void);
-};
+} idle_sampling_loop_state;
 
 static VALUE _native_new(VALUE klass);
-static void reset_state(struct idle_sampling_loop_state *state);
+static void reset_state(idle_sampling_loop_state *state);
 static VALUE _native_idle_sampling_loop(DDTRACE_UNUSED VALUE self, VALUE self_instance);
 static VALUE _native_stop(DDTRACE_UNUSED VALUE self, VALUE self_instance);
 static void *run_idle_sampling_loop(void *state_ptr);
@@ -62,7 +62,7 @@ void collectors_idle_sampling_helper_init(VALUE profiling_module) {
   rb_define_singleton_method(testing_module, "_native_idle_sampling_helper_request_action", _native_idle_sampling_helper_request_action, 1);
 }
 
-// This structure is used to define a Ruby object that stores a pointer to a struct idle_sampling_loop_state
+// This structure is used to define a Ruby object that stores a pointer to a idle_sampling_loop_state
 // See also https://github.com/ruby/ruby/blob/master/doc/extension.rdoc for how this works
 static const rb_data_type_t idle_sampling_helper_typed_data = {
   .wrap_struct_name = "Datadog::Profiling::Collectors::IdleSamplingHelper",
@@ -76,7 +76,7 @@ static const rb_data_type_t idle_sampling_helper_typed_data = {
 };
 
 static VALUE _native_new(VALUE klass) {
-  struct idle_sampling_loop_state *state = ruby_xcalloc(1, sizeof(struct idle_sampling_loop_state));
+  idle_sampling_loop_state *state = ruby_xcalloc(1, sizeof(idle_sampling_loop_state));
 
   // Note: Any exceptions raised from this note until the TypedData_Wrap_Struct call will lead to the state memory
   // being leaked.
@@ -90,7 +90,7 @@ static VALUE _native_new(VALUE klass) {
   return TypedData_Wrap_Struct(klass, &idle_sampling_helper_typed_data, state);
 }
 
-static void reset_state(struct idle_sampling_loop_state *state) {
+static void reset_state(idle_sampling_loop_state *state) {
   state->wakeup_mutex = (pthread_mutex_t) PTHREAD_MUTEX_INITIALIZER;
   state->wakeup = (pthread_cond_t) PTHREAD_COND_INITIALIZER;
   state->requested_action = ACTION_WAIT;
@@ -101,17 +101,17 @@ static void reset_state(struct idle_sampling_loop_state *state) {
 // a pristine state before recreating the worker thread (this includes resetting the mutex in case it was left
 // locked halfway through the VM forking)
 static VALUE _native_reset(DDTRACE_UNUSED VALUE self, VALUE self_instance) {
-  struct idle_sampling_loop_state *state;
-  TypedData_Get_Struct(self_instance, struct idle_sampling_loop_state, &idle_sampling_helper_typed_data, state);
+  idle_sampling_loop_state *state;
+  TypedData_Get_Struct(self_instance, idle_sampling_loop_state, &idle_sampling_helper_typed_data, state);
 
   reset_state(state);
 
   return Qtrue;
 }
 
 static VALUE _native_idle_sampling_loop(DDTRACE_UNUSED VALUE self, VALUE self_instance) {
-  struct idle_sampling_loop_state *state;
-  TypedData_Get_Struct(self_instance, struct idle_sampling_loop_state, &idle_sampling_helper_typed_data, state);
+  idle_sampling_loop_state *state;
+  TypedData_Get_Struct(self_instance, idle_sampling_loop_state, &idle_sampling_helper_typed_data, state);
 
   // Release GVL and run the loop waiting for requests
   rb_thread_call_without_gvl(run_idle_sampling_loop, state, interrupt_idle_sampling_loop, state);
@@ -120,7 +120,7 @@ static VALUE _native_idle_sampling_loop(DDTRACE_UNUSED VALUE self, VALUE self_in
 }
 
 static void *run_idle_sampling_loop(void *state_ptr) {
-  struct idle_sampling_loop_state *state = (struct idle_sampling_loop_state *) state_ptr;
+  idle_sampling_loop_state *state = (idle_sampling_loop_state *) state_ptr;
   int error = 0;
 
   while (true) {
@@ -164,7 +164,7 @@ static void *run_idle_sampling_loop(void *state_ptr) {
 }
 
 static void interrupt_idle_sampling_loop(void *state_ptr) {
-  struct idle_sampling_loop_state *state = (struct idle_sampling_loop_state *) state_ptr;
+  idle_sampling_loop_state *state = (idle_sampling_loop_state *) state_ptr;
   int error = 0;
 
   // Note about the error handling in this situation: Something bad happening at this stage is really really awkward to
@@ -189,8 +189,8 @@ static void interrupt_idle_sampling_loop(void *state_ptr) {
 }
 
 static VALUE _native_stop(DDTRACE_UNUSED VALUE self, VALUE self_instance) {
-  struct idle_sampling_loop_state *state;
-  TypedData_Get_Struct(self_instance, struct idle_sampling_loop_state, &idle_sampling_helper_typed_data, state);
+  idle_sampling_loop_state *state;
+  TypedData_Get_Struct(self_instance, idle_sampling_loop_state, &idle_sampling_helper_typed_data, state);
 
   ENFORCE_SUCCESS_GVL(pthread_mutex_lock(&state->wakeup_mutex));
   state->requested_action = ACTION_STOP;
@@ -204,12 +204,12 @@ static VALUE _native_stop(DDTRACE_UNUSED VALUE self, VALUE self_instance) {
 
 // Assumption: Function gets called without the global VM lock
 void idle_sampling_helper_request_action(VALUE self_instance, void (*run_action_function)(void)) {
-  struct idle_sampling_loop_state *state;
+  idle_sampling_loop_state *state;
   if (!rb_typeddata_is_kind_of(self_instance, &idle_sampling_helper_typed_data)) {
     grab_gvl_and_raise(rb_eTypeError, "Wrong argument for idle_sampling_helper_request_action");
   }
   // This should never fail the the above check passes
-  TypedData_Get_Struct(self_instance, struct idle_sampling_loop_state, &idle_sampling_helper_typed_data, state);
+  TypedData_Get_Struct(self_instance, idle_sampling_loop_state, &idle_sampling_helper_typed_data, state);
 
   ENFORCE_SUCCESS_NO_GVL(pthread_mutex_lock(&state->wakeup_mutex));
   if (state->requested_action == ACTION_WAIT) {

ext/datadog_profiling_native_extension/collectors_idle_sampling_helper.c

@@ -14,11 +14,11 @@
 static VALUE missing_string = Qnil;
 
 // Used as scratch space during sampling
-struct sampling_buffer {
+struct sampling_buffer { // Note: typedef'd in the header to sampling_buffer
   uint16_t max_frames;
   ddog_prof_Location *locations;
   frame_info *stack_buffer;
-}; // Note: typedef'd in the header to sampling_buffer
+};
 
 static VALUE _native_sample(int argc, VALUE *argv, DDTRACE_UNUSED VALUE _self);
 static VALUE native_sample_do(VALUE args);
@@ -44,15 +44,15 @@ void collectors_stack_init(VALUE profiling_module) {
   rb_global_variable(&missing_string);
 }
 
-struct native_sample_args {
+typedef struct {
   VALUE in_gc;
   VALUE recorder_instance;
   sample_values values;
   sample_labels labels;
   VALUE thread;
   ddog_prof_Location *locations;
   sampling_buffer *buffer;
-};
+} native_sample_args;
 
 // This method exists only to enable testing Datadog::Profiling::Collectors::Stack behavior using RSpec.
 // It SHOULD NOT be used for other purposes.
@@ -123,7 +123,7 @@ static VALUE _native_sample(int argc, VALUE *argv, DDTRACE_UNUSED VALUE _self) {
 
   ddog_prof_Slice_Label slice_labels = {.ptr = labels, .len = labels_count};
 
-  struct native_sample_args args_struct = {
+  native_sample_args args_struct = {
     .in_gc = in_gc,
     .recorder_instance = recorder_instance,
     .values = values,
@@ -137,7 +137,7 @@ static VALUE _native_sample(int argc, VALUE *argv, DDTRACE_UNUSED VALUE _self) {
 }
 
 static VALUE native_sample_do(VALUE args) {
-  struct native_sample_args *args_struct = (struct native_sample_args *) args;
+  native_sample_args *args_struct = (native_sample_args *) args;
 
   if (args_struct->in_gc == Qtrue) {
     record_placeholder_stack(
@@ -160,7 +160,7 @@ static VALUE native_sample_do(VALUE args) {
 }
 
 static VALUE native_sample_ensure(VALUE args) {
-  struct native_sample_args *args_struct = (struct native_sample_args *) args;
+  native_sample_args *args_struct = (native_sample_args *) args;
 
   ruby_xfree(args_struct->locations);
   sampling_buffer_free(args_struct->buffer);