Merge pull request #276 from ebkalderon/fix-jsonl-parse-errors

Fix several regressions from adoption of JSONL file format

Author: emeryberger

.github/workflows/ci.yml

@@ -39,15 +39,23 @@ jobs:
       run: |
         cd build
         cmake .. -DBUILD_BENCHMARKS=ON -DCMAKE_BUILD_TYPE=RelWithDebInfo
-        make -j$(nproc) toy lock_test
+        make -j$(nproc) toy lock_test kmeans
 
     - name: Verify build artifacts
       run: |
         test -f build/libcoz/libcoz.so
         test -x build/benchmarks/toy/toy
         test -x build/benchmarks/lock_test/lock_test
+        test -x build/benchmarks/kmeans/kmeans
         echo "Build artifacts verified"
 
+    - name: Run unit tests
+      run: |
+        cd build
+        ctest --output-on-failure -j$(nproc)
+        rm -f profile.jsonl
+        echo "Unit tests passed"
+
     - name: Run toy benchmark with coz
       run: |
         cd build
@@ -115,7 +123,7 @@ jobs:
                 obj = json.loads(line)
                 if obj.get('type') == 'experiment':
                     current_exp = obj
-                elif obj.get('type') == 'throughput_point' and current_exp:
+                elif obj.get('type') == 'throughput-point' and current_exp:
                     current_exp['delta'] = float(obj['delta'])
                     experiments.append(current_exp)
                     current_exp = None
@@ -192,7 +200,7 @@ jobs:
                 obj = json.loads(line)
                 if obj.get('type') == 'experiment':
                     current_exp = obj
-                elif obj.get('type') == 'throughput_point' and current_exp:
+                elif obj.get('type') == 'throughput-point' and current_exp:
                     current_exp['delta'] = float(obj['delta'])
                     experiments.append(current_exp)
                     current_exp = None

.gitignore

@@ -39,7 +39,9 @@ libelfin/
 
 viewer/node_modules
 rust/Cargo.lock
-rust/target
+rust/target/
+tests/rust_filter/Cargo.lock
+tests/rust_filter/target/
 
 # CMake generated files
 CMakeLists.txt.user

benchmarks/check-output.sh

@@ -1,11 +1,11 @@
 #!/bin/sh
 
 set -e
-rm -f profile.coz
+rm -f profile.jsonl
 
 $@
 
-grep -q "time=" profile.coz || { echo failure: valid profile.coz not generated; exit 1; }
-grep -q "throughput-point" profile.coz || { echo failure: throughput-point not found in profile; exit 1; }
-grep -q -P "samples\tlocation=" profile.coz || { echo failure: samples not found in profile; exit 1; }
-echo success: benchmark generated valid profile.coz
+grep -q '"time":' profile.jsonl || { echo failure: valid profile.jsonl not generated; exit 1; }
+grep -q '"throughput-point"' profile.jsonl || { echo failure: throughput-point not found in profile; exit 1; }
+grep -q -P '{"type":"samples","location":' profile.jsonl || { echo failure: samples not found in profile; exit 1; }
+echo success: benchmark generated valid profile.jsonl

coz

@@ -236,7 +236,7 @@ def parse_profile(profile_path, include_raw=False):
             'selected_samples': int(record.get('selected_samples', 0))
           }
           experiment_count += 1
-        elif record_type == 'throughput_point':
+        elif record_type == 'throughput-point':
           if experiment:
             selected = experiment['selected']
             speedup = experiment['speedup']

libcoz/profiler.cpp

@@ -400,7 +400,7 @@ void profiler::profiler_thread(spinlock& l) {
       for(const auto& s : saved_throughput_points) {
         size_t delta = s->get_delta();
         if(_json_output) {
-          output << "{\"type\":\"throughput_point\",\"name\":\"" << json_escape(s->get_name()) << "\","
+          output << "{\"type\":\"throughput-point\",\"name\":\"" << json_escape(s->get_name()) << "\","
                  << "\"delta\":" << delta << "}\n";
         } else {
           s->log(output);
@@ -409,7 +409,7 @@ void profiler::profiler_thread(spinlock& l) {
 
       for(const auto& s : saved_latency_points) {
         if(_json_output) {
-          output << "{\"type\":\"latency_point\",\"name\":\"" << json_escape(s->get_name()) << "\","
+          output << "{\"type\":\"latency-point\",\"name\":\"" << json_escape(s->get_name()) << "\","
                  << "\"arrivals\":" << s->get_begin_delta() << ","
                  << "\"departures\":" << s->get_end_delta() << ","
                  << "\"difference\":" << s->get_difference() << "}\n";

tests/run_dwarf_scope_test.sh

@@ -30,13 +30,12 @@ run_profile() {
 
 PROFILE_DEFAULT="$OUTDIR/profile-default.coz"
 run_profile "$PROFILE_DEFAULT"
-
-grep -Eq '^samples[[:space:]]+location=.*dwarf_scope_test\.cpp' "$PROFILE_DEFAULT"
-if grep -Eq '/usr/(include|lib)/' "$PROFILE_DEFAULT"; then
-  echo "unexpected system source in default profile" >&2
-  exit 1
-fi
+grep -Eq '^{"type":"samples","location":.*dwarf_scope_test\.cpp' "$PROFILE_DEFAULT"
 
 PROFILE_FILTERED="$OUTDIR/profile-filtered.coz"
 COZ_FILTER_SYSTEM=1 run_profile "$PROFILE_FILTERED"
-grep -Eq '^samples[[:space:]]+location=.*dwarf_scope_test\.cpp' "$PROFILE_FILTERED"
+grep -Eq '^{"type":"samples","location":.*dwarf_scope_test\.cpp' "$PROFILE_FILTERED"
+if grep -Eq '/usr/(include|lib)/' "$PROFILE_FILTERED"; then
+  echo "unexpected system source in filtered profile" >&2
+  exit 1
+fi

viewer/js/profile.js

@@ -40,13 +40,13 @@ interface Experiment {
 }
 
 interface ThroughputPoint {
-  type: 'throughput-point' | 'progress-point' | 'throughput_point';
+  type: 'throughput-point' | 'throughput_point' | 'progress-point';
   name: string;
   delta: number;
 }
 
 interface LatencyPoint {
-  type: 'latency-point';
+  type: 'latency-point' | 'latency_point';
   name: string;
   arrivals: number;
   departures: number;
@@ -1023,11 +1023,11 @@ class Profile {
         } else {
           experiment = entry;
         }
-      } else if (entry.type === 'throughput-point' || entry.type === 'progress-point' || entry.type === 'throughput_point') {
+      } else if (entry.type === 'throughput-point' || entry.type === 'throughput_point' || entry.type === 'progress-point') {
         if (experiment !== null) {
           this.addThroughputMeasurement(experiment, entry);
         }
-      } else if (entry.type === 'latency-point') {
+      } else if (entry.type === 'latency-point' || entry.type === 'latency_point') {
         if (experiment !== null) {
           this.addLatencyMeasurement(experiment, entry);
         }