Make MultivariateTimeSeries a non-template class (#4723)

vickyliu-go4it · web-flow · commit 074e46f26fab · 2023-01-18T16:58:16.000+01:00
* Make MultivariateTimeSeries a non-template class 1st step to make `GraphTrack` not templated based on the data series `Dimension`. Bug: #4534 Test: unit tests * fixup
diff --git a/src/OrbitGl/BasicPageFaultsTrack.cpp b/src/OrbitGl/BasicPageFaultsTrack.cpp
@@ -11,7 +11,6 @@
 #include <string_view>
 
 #include "OrbitGl/GlCanvas.h"
-#include "OrbitGl/MultivariateTimeSeries.h"
 #include "OrbitGl/PickingManager.h"
 
 namespace orbit_gl {
diff --git a/src/OrbitGl/CMakeLists.txt b/src/OrbitGl/CMakeLists.txt
@@ -125,6 +125,7 @@ target_sources(
           MajorPageFaultsTrack.cpp
           MemoryTrack.cpp
           MinorPageFaultsTrack.cpp
+          MultivariateTimeSeries.cpp
           OpenGlBatcher.cpp
           OrbitApp.cpp
           QtTextRenderer.cpp
diff --git a/src/OrbitGl/GraphTrack.cpp b/src/OrbitGl/GraphTrack.cpp
@@ -47,7 +47,8 @@ GraphTrack<Dimension>::GraphTrack(CaptureViewElement* parent,
                                   const orbit_client_data::ModuleManager* module_manager,
                                   const orbit_client_data::CaptureData* capture_data)
     : Track(parent, timeline_info, viewport, layout, module_manager, capture_data),
-      series_{series_names, series_value_decimal_digits, std::move(series_value_units)} {}
+      series_{std::vector(series_names.begin(), series_names.end()), series_value_decimal_digits,
+              std::move(series_value_units)} {}
 
 template <size_t Dimension>
 bool GraphTrack<Dimension>::HasLegend() const {
@@ -136,14 +137,14 @@ float GraphTrack<Dimension>::GetLabelYFromValues(
 template <size_t Dimension>
 std::string GraphTrack<Dimension>::GetLabelTextFromValues(
     const std::array<double, Dimension>& values) const {
-  const std::array<std::string, Dimension>& series_names = series_.GetSeriesNames();
+  const std::vector<std::string>& series_names = series_.GetSeriesNames();
   std::optional<uint8_t> value_decimal_digits = series_.GetValueDecimalDigits();
   std::string value_unit = series_.GetValueUnit();
   std::string text;
   std::string_view delimiter = "";
   for (int i = Dimension - 1; i >= 0; i--) {
     std::string formatted_name =
-        series_names[i].empty() ? "" : absl::StrFormat("%s: ", series_names[i]);
+        series_names.at(i).empty() ? "" : absl::StrFormat("%s: ", series_names.at(i));
     std::string formatted_value =
         absl::StrFormat("%.*f", value_decimal_digits.value_or(6), values[i]);
     absl::StrAppend(&text, delimiter, formatted_name, formatted_value, value_unit);
@@ -175,8 +176,12 @@ void GraphTrack<Dimension>::DrawMouseLabel(PrimitiveAssembler& primitive_assembl
   const Color transparent_white(255, 255, 255, 180);
 
   uint64_t current_mouse_time_ns = draw_context.current_mouse_tick.value();
-  const std::array<double, Dimension>& values =
-      series_.GetPreviousOrFirstEntry(current_mouse_time_ns);
+
+  // TODO(vickyliu): remove this vector to array conversion after changing to not use array.
+  const std::vector<double>& values_tmp = series_.GetPreviousOrFirstEntry(current_mouse_time_ns);
+  ORBIT_CHECK(values_tmp.size() == Dimension);
+  std::array<double, Dimension> values;
+  std::copy_n(values_tmp.begin(), Dimension, values.begin());
   uint64_t first_time = series_.StartTimeInNs();
   uint64_t label_time = std::max(current_mouse_time_ns, first_time);
   Vec2 target_point_pos{timeline_info_->GetWorldFromTick(label_time), GetLabelYFromValues(values)};
@@ -225,7 +230,7 @@ void GraphTrack<Dimension>::DrawMouseLabel(PrimitiveAssembler& primitive_assembl
 template <size_t Dimension>
 void GraphTrack<Dimension>::DrawLegend(PrimitiveAssembler& primitive_assembler,
                                        TextRenderer& text_renderer,
-                                       const std::array<std::string, Dimension>& series_names,
+                                       absl::Span<const std::string> series_names,
                                        const Color& legend_text_color) {
   const float space_between_legend_symbol_and_text = layout_->GetGenericFixedSpacerWidth();
   const float space_between_legend_entries = layout_->GetGenericFixedSpacerWidth() * 2;
@@ -244,13 +249,13 @@ void GraphTrack<Dimension>::DrawLegend(PrimitiveAssembler& primitive_assembler,
     x0 += legend_symbol_width + space_between_legend_symbol_and_text;
 
     const float legend_text_width =
-        text_renderer.GetStringWidth(series_names[i].c_str(), font_size);
+        text_renderer.GetStringWidth(series_names.at(i).c_str(), font_size);
     const Vec2 legend_text_box_size(legend_text_width, layout_->GetTextBoxHeight());
 
     TextRenderer::TextFormatting formatting{font_size, legend_text_color, legend_text_box_size[0]};
     formatting.valign = TextRenderer::VAlign::Middle;
 
-    text_renderer.AddText(series_names[i].c_str(), x0, y0 + legend_symbol_height / 2.f, text_z,
+    text_renderer.AddText(series_names.at(i).c_str(), x0, y0 + legend_symbol_height / 2.f, text_z,
                           formatting);
     auto user_data = std::make_unique<PickingUserData>(
         nullptr, [this, i](PickingId /*id*/) { return GetLegendTooltips(i); });
@@ -271,6 +276,7 @@ void GraphTrack<Dimension>::DrawSeries(PrimitiveAssembler& primitive_assembler,
   double inverse_value_range = GetInverseOfGraphValueRange();
   auto current_it = entries.begin();
   auto last_it = std::prev(entries.end());
+  ORBIT_CHECK(current_it->second.size() == Dimension);
 
   GraphTrackDataAggregator<Dimension> aggr;
 
@@ -281,7 +287,7 @@ void GraphTrack<Dimension>::DrawSeries(PrimitiveAssembler& primitive_assembler,
   // We skip the last element because we can't calculate time passed between last element
   // and the next one.
   while (current_it != last_it) {
-    std::array<double, Dimension> cumulative_values{current_it->second};
+    std::vector<double> cumulative_values = current_it->second;
     std::partial_sum(cumulative_values.begin(), cumulative_values.end(), cumulative_values.begin());
     // For the stacked graph, computing y positions from the normalized values results in some
     // floating error. Event if the sum of values is fixed, the top of the stacked graph may not be
diff --git a/src/OrbitGl/LineGraphTrack.cpp b/src/OrbitGl/LineGraphTrack.cpp
@@ -24,7 +24,7 @@ namespace orbit_gl {
 
 template <size_t Dimension>
 [[nodiscard]] static std::array<float, Dimension> GetNormalizedValues(
-    const std::array<double, Dimension>& values, double min, double inverse_value_range) {
+    absl::Span<const double> values, double min, double inverse_value_range) {
   std::array<float, Dimension> normalized_values;
   std::transform(values.begin(), values.end(), normalized_values.begin(),
                  [min, inverse_value_range](double value) {
@@ -41,7 +41,7 @@ float LineGraphTrack<Dimension>::GetLabelYFromValues(
   double min = this->GetGraphMinValue();
   double inverse_value_range = this->GetInverseOfGraphValueRange();
   std::array<float, Dimension> normalized_values =
-      GetNormalizedValues(values, min, inverse_value_range);
+      GetNormalizedValues<Dimension>(values, min, inverse_value_range);
   // The label will point to the only value.
   if (Dimension == 1) return base_y - normalized_values[0] * content_height;
 
@@ -63,7 +63,7 @@ void LineGraphTrack<Dimension>::DrawSeries(PrimitiveAssembler& primitive_assembl
 
   // Normalized values that were last used for drawing.
   std::array<float, Dimension> prev_drawn_values =
-      GetNormalizedValues(curr_iterator->second, min, inverse_value_range);
+      GetNormalizedValues<Dimension>(curr_iterator->second, min, inverse_value_range);
 
   // Normalized values of the last entry we've iterated over.
   std::array<float, Dimension> last_entry_values = prev_drawn_values;
@@ -113,7 +113,7 @@ void LineGraphTrack<Dimension>::DrawSeries(PrimitiveAssembler& primitive_assembl
 
     uint64_t curr_time = curr_iterator->first;
     std::array<float, Dimension> curr_normalized_values =
-        GetNormalizedValues(curr_iterator->second, min, inverse_value_range);
+        GetNormalizedValues<Dimension>(curr_iterator->second, min, inverse_value_range);
 
     if (aggr.GetAccumulatedEntry() == nullptr) {
       aggr.SetEntry(prev_time, curr_time, curr_normalized_values);
diff --git a/src/OrbitGl/MultivariateTimeSeries.cpp b/src/OrbitGl/MultivariateTimeSeries.cpp
@@ -0,0 +1,109 @@
+// Copyright (c) 2023 The Orbit Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "OrbitGl/MultivariateTimeSeries.h"
+
+#include "OrbitBase/Logging.h"
+
+namespace orbit_gl {
+
+MultivariateTimeSeries::MultivariateTimeSeries(std::vector<std::string> series_names,
+                                               uint8_t value_decimal_digits, std::string value_unit)
+    : series_names_{std::move(series_names)},
+      value_decimal_digits_{value_decimal_digits},
+      value_unit_{std::move(value_unit)} {
+  ORBIT_CHECK(!series_names_.empty());
+}
+
+double MultivariateTimeSeries::GetMin() const {
+  absl::MutexLock lock(&mutex_);
+  return min_;
+}
+
+double MultivariateTimeSeries::GetMax() const {
+  absl::MutexLock lock(&mutex_);
+  return max_;
+}
+
+bool MultivariateTimeSeries::IsEmpty() const {
+  absl::MutexLock lock(&mutex_);
+  return time_to_series_values_.empty();
+}
+
+size_t MultivariateTimeSeries::GetTimeToSeriesValuesSize() const {
+  absl::MutexLock lock(&mutex_);
+  return time_to_series_values_.size();
+}
+
+uint64_t MultivariateTimeSeries::StartTimeInNs() const {
+  absl::MutexLock lock(&mutex_);
+  ORBIT_CHECK(!time_to_series_values_.empty());
+  return time_to_series_values_.begin()->first;
+}
+
+uint64_t MultivariateTimeSeries::EndTimeInNs() const {
+  absl::MutexLock lock(&mutex_);
+  ORBIT_CHECK(!time_to_series_values_.empty());
+  return time_to_series_values_.rbegin()->first;
+}
+
+std::vector<double> MultivariateTimeSeries::GetPreviousOrFirstEntry(uint64_t time) const {
+  absl::MutexLock lock(&mutex_);
+  return GetPreviousOrFirstEntryIterator(time)->second;
+}
+
+std::vector<std::pair<uint64_t, std::vector<double>>>
+MultivariateTimeSeries::GetEntriesAffectedByTimeRange(uint64_t min_time, uint64_t max_time) const {
+  absl::MutexLock lock(&mutex_);
+  if (time_to_series_values_.empty() || min_time >= max_time ||
+      min_time >= time_to_series_values_.rbegin()->first ||
+      max_time <= time_to_series_values_.begin()->first) {
+    return {};
+  }
+
+  auto current_it = GetPreviousOrFirstEntryIterator(min_time);
+  auto last_iterator = GetNextOrLastEntryIterator(max_time);
+
+  std::vector<std::pair<uint64_t, std::vector<double>>> result;
+  result.push_back(*current_it);
+  do {
+    ++current_it;
+    result.push_back(*current_it);
+  } while (current_it != last_iterator);
+
+  return result;
+}
+
+void MultivariateTimeSeries::AddValues(uint64_t timestamp_ns, absl::Span<const double> values) {
+  ORBIT_CHECK(values.size() == series_names_.size());
+
+  absl::MutexLock lock(&mutex_);
+  time_to_series_values_[timestamp_ns] = std::vector(values.begin(), values.end());
+  for (double value : values) UpdateMinAndMax(value);
+}
+
+MultivariateTimeSeries::TimeSeriesEntryIter MultivariateTimeSeries::GetPreviousOrFirstEntryIterator(
+    uint64_t time) const {
+  ORBIT_CHECK(!time_to_series_values_.empty());
+
+  auto iterator_lower = time_to_series_values_.upper_bound(time);
+  if (iterator_lower != time_to_series_values_.begin()) --iterator_lower;
+  return iterator_lower;
+}
+
+MultivariateTimeSeries::TimeSeriesEntryIter MultivariateTimeSeries::GetNextOrLastEntryIterator(
+    uint64_t time) const {
+  ORBIT_CHECK(!time_to_series_values_.empty());
+
+  auto iterator_higher = time_to_series_values_.lower_bound(time);
+  if (iterator_higher == time_to_series_values_.end()) --iterator_higher;
+  return iterator_higher;
+}
+
+void MultivariateTimeSeries::UpdateMinAndMax(double value) {
+  max_ = std::max(max_, value);
+  min_ = std::min(min_, value);
+}
+
+}  // namespace orbit_gl
diff --git a/src/OrbitGl/MultivariateTimeSeriesTest.cpp b/src/OrbitGl/MultivariateTimeSeriesTest.cpp
@@ -15,44 +15,41 @@
 
 namespace orbit_gl {
 
-static constexpr uint8_t kDefaultValueDesimalDigits = 6;
-static constexpr const char* kDefaultValueUnits = "";
+const std::vector<std::string> kSeriesNames{"Series A", "Series B", "Series C"};
+static constexpr uint8_t kDefaultValueDecimalDigits = 6;
+static constexpr const char* kDefaultValueUnits = "Unit";
+
+constexpr uint64_t kTimestamp1 = 100;
+constexpr std::array<double, 3> kValues1{1.1, 1.2, 1.3};
+constexpr uint64_t kTimestamp2 = 200;
+constexpr std::array<double, 3> kValues2{2.1, 2.2, 2.3};
+constexpr uint64_t kTimestamp3 = 300;
+constexpr std::array<double, 3> kValues3{3.1, 3.2, 3.3};
+
+void AddTestValuesToSeries(MultivariateTimeSeries& series) {
+  series.AddValues(kTimestamp1, kValues1);
+  series.AddValues(kTimestamp2, kValues2);
+  series.AddValues(kTimestamp3, kValues3);
+}
 
 TEST(MultivariateTimeSeries, BasicSetAndGet) {
-  std::array<std::string, 3> series_names = {"Series A", "Series B", "Series C"};
-  MultivariateTimeSeries<3> series =
-      MultivariateTimeSeries<3>(series_names, kDefaultValueDesimalDigits, kDefaultValueUnits);
-  EXPECT_EQ(series.GetSeriesNames(), series_names);
+  MultivariateTimeSeries series{kSeriesNames, kDefaultValueDecimalDigits, kDefaultValueUnits};
+  EXPECT_EQ(series.GetSeriesNames(), kSeriesNames);
+  EXPECT_EQ(series.GetValueDecimalDigits(), kDefaultValueDecimalDigits);
+  EXPECT_EQ(series.GetValueUnit(), kDefaultValueUnits);
   EXPECT_TRUE(series.IsEmpty());
 
-  uint64_t timestamp_1 = 100;
-  std::array<double, 3> values_1 = {1.1, 1.2, 1.3};
-  uint64_t timestamp_2 = 200;
-  std::array<double, 3> values_2 = {2.1, 2.2, 2.3};
-  uint64_t timestamp_3 = 300;
-  std::array<double, 3> values_3 = {3.1, 3.2, 3.3};
-  series.AddValues(timestamp_1, values_1);
-  series.AddValues(timestamp_2, values_2);
-  series.AddValues(timestamp_3, values_3);
-  EXPECT_FALSE(series.IsEmpty());
-  EXPECT_EQ(series.GetMax(), values_3[2]);
-  EXPECT_EQ(series.GetMin(), values_1[0]);
-  EXPECT_EQ(series.StartTimeInNs(), timestamp_1);
-  EXPECT_EQ(series.EndTimeInNs(), timestamp_3);
+  AddTestValuesToSeries(series);
+  EXPECT_EQ(series.GetTimeToSeriesValuesSize(), 3);
+  EXPECT_EQ(series.GetMin(), kValues1[0]);
+  EXPECT_EQ(series.GetMax(), kValues3[2]);
+  EXPECT_EQ(series.StartTimeInNs(), kTimestamp1);
+  EXPECT_EQ(series.EndTimeInNs(), kTimestamp3);
 }
 
 TEST(MultivariateTimeSeries, GetPreviousOrFirstEntry) {
-  MultivariateTimeSeries<3> series = MultivariateTimeSeries<3>(
-      {"Series A", "Series B", "Series C"}, kDefaultValueDesimalDigits, kDefaultValueUnits);
-  uint64_t timestamp_1 = 100;
-  std::array<double, 3> values_1 = {1.1, 1.2, 1.3};
-  uint64_t timestamp_2 = 200;
-  std::array<double, 3> values_2 = {2.1, 2.2, 2.3};
-  uint64_t timestamp_3 = 300;
-  std::array<double, 3> values_3 = {3.1, 3.2, 3.3};
-  series.AddValues(timestamp_1, values_1);
-  series.AddValues(timestamp_2, values_2);
-  series.AddValues(timestamp_3, values_3);
+  MultivariateTimeSeries series{kSeriesNames, kDefaultValueDecimalDigits, kDefaultValueUnits};
+  AddTestValuesToSeries(series);
 
   {
     uint64_t timestamp_before_first_time = 50;
@@ -74,17 +71,8 @@ TEST(MultivariateTimeSeries, GetPreviousOrFirstEntry) {
 }
 
 TEST(MultivariateTimeSeries, GetEntriesAffectedByTimeRange) {
-  MultivariateTimeSeries<3> series = MultivariateTimeSeries<3>(
-      {"Series A", "Series B", "Series C"}, kDefaultValueDesimalDigits, kDefaultValueUnits);
-  uint64_t timestamp_1 = 100;
-  std::array<double, 3> values_1 = {1.1, 1.2, 1.3};
-  uint64_t timestamp_2 = 200;
-  std::array<double, 3> values_2 = {2.1, 2.2, 2.3};
-  uint64_t timestamp_3 = 300;
-  std::array<double, 3> values_3 = {3.1, 3.2, 3.3};
-  series.AddValues(timestamp_1, values_1);
-  series.AddValues(timestamp_2, values_2);
-  series.AddValues(timestamp_3, values_3);
+  MultivariateTimeSeries series{kSeriesNames, kDefaultValueDecimalDigits, kDefaultValueUnits};
+  AddTestValuesToSeries(series);
 
   {
     // Test the case that min_time >= max time
@@ -108,24 +96,13 @@ TEST(MultivariateTimeSeries, GetEntriesAffectedByTimeRange) {
     uint64_t max_time = 400;
     auto entries = series.GetEntriesAffectedByTimeRange(min_time, max_time);
     ASSERT_EQ(entries.size(), 3);
-    EXPECT_EQ(entries[0].first, timestamp_1);
+    EXPECT_EQ(entries[0].first, kTimestamp1);
     EXPECT_THAT(entries[0].second, testing::ElementsAre(1.1, 1.2, 1.3));
-    EXPECT_EQ(entries[1].first, timestamp_2);
+    EXPECT_EQ(entries[1].first, kTimestamp2);
     EXPECT_THAT(entries[1].second, testing::ElementsAre(2.1, 2.2, 2.3));
-    EXPECT_EQ(entries[2].first, timestamp_3);
+    EXPECT_EQ(entries[2].first, kTimestamp3);
     EXPECT_THAT(entries[2].second, testing::ElementsAre(3.1, 3.2, 3.3));
   }
 }
 
-TEST(MultivariateTimeSeries, GetValueDecimalDigits) {
-  MultivariateTimeSeries<3> series =
-      MultivariateTimeSeries<3>({"Series A", "Series B", "Series C"}, 42, kDefaultValueUnits);
-  EXPECT_EQ(series.GetValueDecimalDigits(), 42);
-}
-
-TEST(MultivariateTimeSeries, GetValueUnits) {
-  MultivariateTimeSeries<3> series = MultivariateTimeSeries<3>(
-      {"Series A", "Series B", "Series C"}, kDefaultValueDesimalDigits, "Meeples");
-  EXPECT_EQ(series.GetValueUnit(), "Meeples");
-}
 }  // namespace orbit_gl
diff --git a/src/OrbitGl/include/OrbitGl/GraphTrack.h b/src/OrbitGl/include/OrbitGl/GraphTrack.h
@@ -112,7 +112,7 @@ class GraphTrack : public Track {
                               const DrawContext& draw_context);
   virtual void DrawLegend(orbit_gl::PrimitiveAssembler& primitive_assembler,
                           orbit_gl::TextRenderer& text_renderer,
-                          const std::array<std::string, Dimension>& series_names,
+                          absl::Span<const std::string> series_names,
                           const Color& legend_text_color);
   virtual void DrawSeries(orbit_gl::PrimitiveAssembler& primitive_assembler, uint64_t min_tick,
                           uint64_t max_tick, float z);
@@ -122,7 +122,7 @@ class GraphTrack : public Track {
            std::pow(10, GetNumberOfDecimalDigits());
   }
 
-  MultivariateTimeSeries<Dimension> series_;
+  orbit_gl::MultivariateTimeSeries series_;
 
  private:
   [[nodiscard]] virtual std::string GetLegendTooltips(size_t legend_index) const = 0;
diff --git a/src/OrbitGl/include/OrbitGl/MultivariateTimeSeries.h b/src/OrbitGl/include/OrbitGl/MultivariateTimeSeries.h
