replace ruby hash with C hashtable

ext/datadog_cov/datadog_cov.c

@@ -6,14 +6,12 @@
 struct dd_cov_data
 {
   VALUE root;
-  VALUE coverage;
   st_table *covered_files;
 };
 
 static void dd_cov_mark(void *ptr)
 {
   struct dd_cov_data *dd_cov_data = ptr;
-  rb_gc_mark_movable(dd_cov_data->coverage);
   rb_gc_mark_movable(dd_cov_data->root);
 }
 
@@ -23,18 +21,11 @@ static int free_hashtable_keys(st_data_t key, st_data_t value, st_data_t data)
   return ST_CONTINUE;
 }
 
-// temporary function to inspect hashtable
-static int inspect_hashtable(st_data_t key, st_data_t value, st_data_t data)
-{
-  printf("key: %s\n", (char *)key);
-  return ST_CONTINUE;
-}
-
 static void dd_cov_free(void *ptr)
 {
   struct dd_cov_data *dd_cov_data = ptr;
 
-  st_foreach(dd_cov_data->covered_files, free_hashtable_keys, 0);
+  // st_foreach(dd_cov_data->covered_files, free_hashtable_keys, 0);
   st_free_table(dd_cov_data->covered_files);
 
   xfree(dd_cov_data);
@@ -43,7 +34,6 @@ static void dd_cov_free(void *ptr)
 static void dd_cov_compact(void *ptr)
 {
   struct dd_cov_data *dd_cov_data = ptr;
-  dd_cov_data->coverage = rb_gc_location(dd_cov_data->coverage);
   dd_cov_data->root = rb_gc_location(dd_cov_data->root);
 }
 
@@ -61,9 +51,7 @@ static VALUE dd_cov_allocate(VALUE klass)
   struct dd_cov_data *dd_cov_data;
   VALUE obj = TypedData_Make_Struct(klass, struct dd_cov_data, &dd_cov_data_type, dd_cov_data);
 
-  dd_cov_data->coverage = rb_hash_new();
   dd_cov_data->root = Qnil;
-
   dd_cov_data->covered_files = st_init_strtable();
 
   return obj;
@@ -118,12 +106,7 @@ static void dd_cov_update_line_coverage(rb_event_flag_t event, VALUE data, VALUE
     return;
   }
 
-  // new way to store covered files
   st_insert(dd_cov_data->covered_files, (st_data_t)filename, 1);
-
-  // old way to store covered files
-  VALUE rb_str_source_file = rb_str_new2(filename);
-  rb_hash_aset(dd_cov_data->coverage, rb_str_source_file, Qtrue);
 }
 
 static VALUE dd_cov_start(VALUE self)
@@ -137,6 +120,12 @@ static VALUE dd_cov_start(VALUE self)
   return self;
 }
 
+static int insert_into_result(st_data_t filename, st_data_t _val, st_data_t ary)
+{
+  rb_ary_push((VALUE)ary, rb_str_new2((char *)filename));
+  return ST_CONTINUE;
+}
+
 static VALUE dd_cov_stop(VALUE self)
 {
   // get current thread
@@ -147,13 +136,9 @@ static VALUE dd_cov_stop(VALUE self)
   struct dd_cov_data *dd_cov_data;
   TypedData_Get_Struct(self, struct dd_cov_data, &dd_cov_data_type, dd_cov_data);
 
-  st_foreach(dd_cov_data->covered_files, inspect_hashtable, 0);
-
-  VALUE cov = dd_cov_data->coverage;
-
-  dd_cov_data->coverage = rb_hash_new();
+  VALUE cov = rb_ary_new2(dd_cov_data->covered_files->num_entries);
+  st_foreach(dd_cov_data->covered_files, insert_into_result, cov);
 
-  // freeing these strings makes them unavailable for Ruby too
   // st_foreach(dd_cov_data->covered_files, free_hashtable_keys, 0);
   st_clear(dd_cov_data->covered_files);
 

sig/datadog/ci/itr/coverage/ddcov.rbs

@@ -8,7 +8,7 @@ module Datadog
 
           def start: () -> void
 
-          def stop: () -> Hash[String, untyped]
+          def stop: () -> Array[String]
         end
       end
     end

spec/ddcov/ddcov_spec.rb

@@ -22,7 +22,7 @@ def absolute_path(path)
         expect(calculator.add(1, 2)).to eq(3)
 
         coverage = cov.stop
-        expect(coverage).to eq({})
+        expect(coverage).to eq([])
       end
     end
 
@@ -38,7 +38,7 @@ def absolute_path(path)
         coverage = subject.stop
 
         expect(coverage.size).to eq(3)
-        expect(coverage.keys).to include(
+        expect(coverage).to include(
           absolute_path("calculator/calculator.rb"),
           absolute_path("calculator/operations/add.rb"),
           absolute_path("calculator/operations/subtract.rb")
@@ -51,7 +51,7 @@ def absolute_path(path)
         coverage = subject.stop
         expect(coverage.size).to eq(1)
         # this string will have a bunch of UTF-8 codepoints in it
-        expect(coverage.keys.first).to include("calculator/code_with_")
+        expect(coverage.first).to include("calculator/code_with_")
       end
     end
 
@@ -82,7 +82,7 @@ def absolute_path(path)
         coverage = subject.stop
 
         expect(coverage.size).to eq(2)
-        expect(coverage.keys).to include(
+        expect(coverage).to include(
           absolute_path("calculator/operations/add.rb"),
           absolute_path("calculator/operations/subtract.rb")
         )
@@ -93,13 +93,13 @@ def absolute_path(path)
         expect(calculator.add(1, 2)).to eq(3)
         coverage = subject.stop
         expect(coverage.size).to eq(1)
-        expect(coverage.keys).to include(absolute_path("calculator/operations/add.rb"))
+        expect(coverage).to include(absolute_path("calculator/operations/add.rb"))
 
         subject.start
         expect(calculator.subtract(1, 2)).to eq(-1)
         coverage = subject.stop
         expect(coverage.size).to eq(1)
-        expect(coverage.keys).to include(absolute_path("calculator/operations/subtract.rb"))
+        expect(coverage).to include(absolute_path("calculator/operations/subtract.rb"))
       end
 
       it "does not track coverage when stopped" do
@@ -113,7 +113,7 @@ def absolute_path(path)
         expect(calculator.multiply(1, 2)).to eq(2)
         coverage = subject.stop
         expect(coverage.size).to eq(1)
-        expect(coverage.keys).to include(absolute_path("calculator/operations/multiply.rb"))
+        expect(coverage).to include(absolute_path("calculator/operations/multiply.rb"))
       end
 
       it "does not fail if start called several times" do
@@ -131,16 +131,16 @@ def absolute_path(path)
         coverage = subject.stop
         expect(coverage.size).to eq(1)
 
-        expect(subject.stop).to eq({})
+        expect(subject.stop).to eq([])
       end
 
       it "tracks coverage in mixins" do
         subject.start
         expect(calculator.divide(6, 3)).to eq(2)
         coverage = subject.stop
         expect(coverage.size).to eq(2)
-        expect(coverage.keys).to include(absolute_path("calculator/operations/divide.rb"))
-        expect(coverage.keys).to include(absolute_path("calculator/operations/helpers/calculator_logger.rb"))
+        expect(coverage).to include(absolute_path("calculator/operations/divide.rb"))
+        expect(coverage).to include(absolute_path("calculator/operations/helpers/calculator_logger.rb"))
       end
 
       context "multi threaded execution" do
@@ -167,8 +167,8 @@ def thread_local_cov
 
             coverage = cov.stop
             expect(coverage.size).to eq(2)
-            expect(coverage.keys).to include(absolute_path("calculator/operations/add.rb"))
-            expect(coverage.keys).to include(absolute_path("calculator/operations/multiply.rb"))
+            expect(coverage).to include(absolute_path("calculator/operations/add.rb"))
+            expect(coverage).to include(absolute_path("calculator/operations/multiply.rb"))
           end
 
           t2 = Thread.new do
@@ -185,7 +185,7 @@ def thread_local_cov
 
             coverage = cov.stop
             expect(coverage.size).to eq(1)
-            expect(coverage.keys).to include(absolute_path("calculator/operations/subtract.rb"))
+            expect(coverage).to include(absolute_path("calculator/operations/subtract.rb"))
           end
 
           [t1, t2].each(&:join)