trace_processor: split up counters table

This CL splits up the counters table into two tables: counter_definitions
and counter_values. We also add a view to make the change transparent to
current callers so we don't need to change any users of trace processor.

This change should allow for more efficient queries
from the UI which only wants the definitions to be able to create the
tracks. Moreover, even on the counters view, if queries are done on name,
ref and ref type, we can do this a lot more efficiently on the definitions.

For example on a 80MB trace (b/124495829) with just memory counters
(i.e. no sched) and the following query (very similar to those done
both in the UI and in benchmarking metrics code)
select
  ts,
  lead(ts) over (partition by ref_type order by ts) as ts_end,
  value
from counters
where name = 'SwapCached' and ref = 0

we get the following performance numbers:
Old code: 263.902 ms
New code (using the view): 107.088 ms (2.46x speedup)

In query the UI does on startup (and called out in b/124495829):
select
  name,
  ref,
  ref_type
from counters
where ref is not null
group by ref, ref_type, name

we get the following performance numbers:
Old code: 11020.611 ms
New code (using the view): 29076.535 ms (2.63x slow down)
New code (directly querying counter_definitions): 48.554 ms (226x speedup)

A follow up CL will further improve the speed of the join operation in view
and filters in general significantly.

Bug: 124495829
Change-Id: I57cf1be328364d53bd6e65eeb4434d015df86981
diff --git a/Android.bp b/Android.bp
index 256e440..e0ba868 100644
--- a/Android.bp
+++ b/Android.bp
@@ -3116,7 +3116,8 @@
     "src/trace_processor/args_table.cc",
     "src/trace_processor/args_tracker.cc",
     "src/trace_processor/clock_tracker.cc",
-    "src/trace_processor/counters_table.cc",
+    "src/trace_processor/counter_definitions_table.cc",
+    "src/trace_processor/counter_values_table.cc",
     "src/trace_processor/event_tracker.cc",
     "src/trace_processor/filtered_row_index.cc",
     "src/trace_processor/ftrace_descriptors.cc",
diff --git a/src/trace_processor/BUILD.gn b/src/trace_processor/BUILD.gn
index cdfdf27..57592b0 100644
--- a/src/trace_processor/BUILD.gn
+++ b/src/trace_processor/BUILD.gn
@@ -52,8 +52,10 @@
     "chunked_trace_reader.h",
     "clock_tracker.cc",
     "clock_tracker.h",
-    "counters_table.cc",
-    "counters_table.h",
+    "counter_definitions_table.cc",
+    "counter_definitions_table.h",
+    "counter_values_table.cc",
+    "counter_values_table.h",
     "event_tracker.cc",
     "event_tracker.h",
     "filtered_row_index.cc",
@@ -181,7 +183,6 @@
   testonly = true
   sources = [
     "clock_tracker_unittest.cc",
-    "counters_table_unittest.cc",
     "event_tracker_unittest.cc",
     "filtered_row_index_unittest.cc",
     "ftrace_utils_unittest.cc",
diff --git a/src/trace_processor/args_tracker.cc b/src/trace_processor/args_tracker.cc
index f01f2d0..af279c8 100644
--- a/src/trace_processor/args_tracker.cc
+++ b/src/trace_processor/args_tracker.cc
@@ -61,8 +61,8 @@
       case TableId::kRawEvents:
         storage->mutable_raw_events()->set_arg_set_id(pair.second, set_id);
         break;
-      case TableId::kCounters:
-        storage->mutable_counters()->set_arg_set_id(pair.second, set_id);
+      case TableId::kCounterValues:
+        storage->mutable_counter_values()->set_arg_set_id(pair.second, set_id);
         break;
       case TableId::kInstants:
         storage->mutable_instants()->set_arg_set_id(pair.second, set_id);
diff --git a/src/trace_processor/counter_definitions_table.cc b/src/trace_processor/counter_definitions_table.cc
new file mode 100644
index 0000000..860f9aa
--- /dev/null
+++ b/src/trace_processor/counter_definitions_table.cc
@@ -0,0 +1,180 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "src/trace_processor/counter_definitions_table.h"
+
+namespace perfetto {
+namespace trace_processor {
+
+CounterDefinitionsTable::CounterDefinitionsTable(sqlite3*,
+                                                 const TraceStorage* storage)
+    : storage_(storage) {
+  ref_types_.resize(RefType::kRefMax);
+  ref_types_[RefType::kRefNoRef] = "";
+  ref_types_[RefType::kRefUtid] = "utid";
+  ref_types_[RefType::kRefCpuId] = "cpu";
+  ref_types_[RefType::kRefIrq] = "irq";
+  ref_types_[RefType::kRefSoftIrq] = "softirq";
+  ref_types_[RefType::kRefUpid] = "upid";
+  ref_types_[RefType::kRefUtidLookupUpid] = "upid";
+}
+
+void CounterDefinitionsTable::RegisterTable(sqlite3* db,
+                                            const TraceStorage* storage) {
+  Table::Register<CounterDefinitionsTable>(db, storage, "counter_definitions");
+}
+
+StorageSchema CounterDefinitionsTable::CreateStorageSchema() {
+  const auto& cs = storage_->counter_definitions();
+  return StorageSchema::Builder()
+      .AddColumn<RowColumn>("counter_id")
+      .AddStringColumn("name", &cs.name_ids(), &storage_->string_pool())
+      .AddColumn<RefColumn>("ref", &cs.refs(), &cs.types(), storage_)
+      .AddStringColumn("ref_type", &cs.types(), &ref_types_)
+      .Build({"counter_id"});
+}
+
+uint32_t CounterDefinitionsTable::RowCount() {
+  return storage_->counter_definitions().size();
+}
+
+int CounterDefinitionsTable::BestIndex(const QueryConstraints& qc,
+                                       BestIndexInfo* info) {
+  info->estimated_cost = EstimateCost(qc);
+
+  // Only the string columns are handled by SQLite
+  size_t name_index = schema().ColumnIndexFromName("name");
+  size_t ref_type_index = schema().ColumnIndexFromName("ref_type");
+  info->order_by_consumed = true;
+  for (size_t i = 0; i < qc.constraints().size(); i++) {
+    auto col = static_cast<size_t>(qc.constraints()[i].iColumn);
+    info->omit[i] = col != name_index && col != ref_type_index;
+  }
+
+  return SQLITE_OK;
+}
+
+uint32_t CounterDefinitionsTable::EstimateCost(const QueryConstraints& qc) {
+  // If there is a constraint on the counter id, we can efficiently filter
+  // to a single row.
+  if (HasEqConstraint(qc, "counter_id"))
+    return 1;
+
+  auto eq_name = HasEqConstraint(qc, "name");
+  auto eq_ref = HasEqConstraint(qc, "ref");
+  auto eq_ref_type = HasEqConstraint(qc, "ref_type");
+
+  // If there is a constraint on all three columns, we are going to only return
+  // exaclty one row for sure so make the cost 1.
+  if (eq_name && eq_ref && eq_ref_type)
+    return 1;
+  else if (eq_name && eq_ref)
+    return 10;
+  else if (eq_name)
+    return 100;
+  return RowCount();
+}
+
+CounterDefinitionsTable::RefColumn::RefColumn(std::string col_name,
+                                              const std::deque<int64_t>* refs,
+                                              const std::deque<RefType>* types,
+                                              const TraceStorage* storage)
+    : StorageColumn(col_name, false /* hidden */),
+      refs_(refs),
+      types_(types),
+      storage_(storage) {}
+
+void CounterDefinitionsTable::RefColumn::ReportResult(sqlite3_context* ctx,
+                                                      uint32_t row) const {
+  auto ref = (*refs_)[row];
+  auto type = (*types_)[row];
+  if (type == RefType::kRefUtidLookupUpid) {
+    auto upid = storage_->GetThread(static_cast<uint32_t>(ref)).upid;
+    if (upid.has_value()) {
+      sqlite_utils::ReportSqliteResult(ctx, upid.value());
+    } else {
+      sqlite3_result_null(ctx);
+    }
+  } else {
+    sqlite_utils::ReportSqliteResult(ctx, ref);
+  }
+}
+
+CounterDefinitionsTable::RefColumn::Bounds
+CounterDefinitionsTable::RefColumn::BoundFilter(int, sqlite3_value*) const {
+  return Bounds{};
+}
+
+void CounterDefinitionsTable::RefColumn::Filter(int op,
+                                                sqlite3_value* value,
+                                                FilteredRowIndex* index) const {
+  bool op_is_null = sqlite_utils::IsOpIsNull(op);
+  auto predicate = sqlite_utils::CreateNumericPredicate<int64_t>(op, value);
+  index->FilterRows(
+      [this, predicate, op_is_null](uint32_t row) PERFETTO_ALWAYS_INLINE {
+        auto ref = (*refs_)[row];
+        auto type = (*types_)[row];
+        if (type == RefType::kRefUtidLookupUpid) {
+          auto upid = storage_->GetThread(static_cast<uint32_t>(ref)).upid;
+          // Trying to filter null with any operation we currently handle
+          // should return false.
+          return upid.has_value() ? predicate(upid.value()) : op_is_null;
+        }
+        return predicate(ref);
+      });
+}
+
+CounterDefinitionsTable::RefColumn::Comparator
+CounterDefinitionsTable::RefColumn::Sort(
+    const QueryConstraints::OrderBy& ob) const {
+  if (ob.desc) {
+    return [this](uint32_t f, uint32_t s) { return -CompareRefsAsc(f, s); };
+  }
+  return [this](uint32_t f, uint32_t s) { return CompareRefsAsc(f, s); };
+}
+
+int CounterDefinitionsTable::RefColumn::CompareRefsAsc(uint32_t f,
+                                                       uint32_t s) const {
+  auto ref_f = (*refs_)[f];
+  auto ref_s = (*refs_)[s];
+
+  auto type_f = (*types_)[f];
+  auto type_s = (*types_)[s];
+
+  base::Optional<int64_t> val_f = ref_f;
+  base::Optional<int64_t> val_s = ref_s;
+  if (type_f == RefType::kRefUtidLookupUpid) {
+    val_f = storage_->GetThread(static_cast<uint32_t>(ref_f)).upid;
+  }
+  if (type_s == RefType::kRefUtidLookupUpid) {
+    val_s = storage_->GetThread(static_cast<uint32_t>(ref_s)).upid;
+  }
+
+  bool has_f = val_f.has_value();
+  bool has_s = val_s.has_value();
+  if (has_f && has_s) {
+    return sqlite_utils::CompareValuesAsc(val_f.value(), val_s.value());
+  } else if (has_f && !has_s) {
+    return 1;
+  } else if (!has_f && has_s) {
+    return -1;
+  } else {
+    return 0;
+  }
+}
+
+}  // namespace trace_processor
+}  // namespace perfetto
diff --git a/src/trace_processor/counters_table.h b/src/trace_processor/counter_definitions_table.h
similarity index 83%
rename from src/trace_processor/counters_table.h
rename to src/trace_processor/counter_definitions_table.h
index 84ff4d8..f4f5711 100644
--- a/src/trace_processor/counters_table.h
+++ b/src/trace_processor/counter_definitions_table.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2018 The Android Open Source Project
+ * Copyright (C) 2019 The Android Open Source Project
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -14,8 +14,8 @@
  * limitations under the License.
  */
 
-#ifndef SRC_TRACE_PROCESSOR_COUNTERS_TABLE_H_
-#define SRC_TRACE_PROCESSOR_COUNTERS_TABLE_H_
+#ifndef SRC_TRACE_PROCESSOR_COUNTER_DEFINITIONS_TABLE_H_
+#define SRC_TRACE_PROCESSOR_COUNTER_DEFINITIONS_TABLE_H_
 
 #include <deque>
 #include <memory>
@@ -27,11 +27,11 @@
 namespace perfetto {
 namespace trace_processor {
 
-class CountersTable : public StorageTable {
+class CounterDefinitionsTable : public StorageTable {
  public:
   static void RegisterTable(sqlite3* db, const TraceStorage* storage);
 
-  CountersTable(sqlite3*, const TraceStorage*);
+  CounterDefinitionsTable(sqlite3*, const TraceStorage*);
 
   // StorageTable implementation.
   StorageSchema CreateStorageSchema() override;
@@ -68,10 +68,12 @@
   };
 
  private:
+  uint32_t EstimateCost(const QueryConstraints&);
+
   std::vector<std::string> ref_types_;
   const TraceStorage* const storage_;
 };
 }  // namespace trace_processor
 }  // namespace perfetto
 
-#endif  // SRC_TRACE_PROCESSOR_COUNTERS_TABLE_H_
+#endif  // SRC_TRACE_PROCESSOR_COUNTER_DEFINITIONS_TABLE_H_
diff --git a/src/trace_processor/counter_values_table.cc b/src/trace_processor/counter_values_table.cc
new file mode 100644
index 0000000..07ef5cc
--- /dev/null
+++ b/src/trace_processor/counter_values_table.cc
@@ -0,0 +1,64 @@
+/*
+ * Copyright (C) 9 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "src/trace_processor/counter_values_table.h"
+
+namespace perfetto {
+namespace trace_processor {
+
+CounterValuesTable::CounterValuesTable(sqlite3*, const TraceStorage* storage)
+    : storage_(storage) {}
+
+void CounterValuesTable::RegisterTable(sqlite3* db,
+                                       const TraceStorage* storage) {
+  Table::Register<CounterValuesTable>(db, storage, "counter_values");
+}
+
+StorageSchema CounterValuesTable::CreateStorageSchema() {
+  const auto& cs = storage_->counter_values();
+  return StorageSchema::Builder()
+      .AddColumn<IdColumn>("id", TableId::kCounterValues)
+      .AddNumericColumn("counter_id", &cs.counter_ids())
+      .AddOrderedNumericColumn("ts", &cs.timestamps())
+      .AddNumericColumn("value", &cs.values())
+      .AddNumericColumn("arg_set_id", &cs.arg_set_ids())
+      .Build({"id"});
+}
+
+uint32_t CounterValuesTable::RowCount() {
+  return storage_->counter_values().size();
+}
+
+int CounterValuesTable::BestIndex(const QueryConstraints& qc,
+                                  BestIndexInfo* info) {
+  info->estimated_cost = EstimateCost(qc);
+
+  info->order_by_consumed = true;
+  for (size_t i = 0; i < qc.constraints().size(); i++) {
+    info->omit[i] = true;
+  }
+
+  return SQLITE_OK;
+}
+
+uint32_t CounterValuesTable::EstimateCost(const QueryConstraints& qc) {
+  if (HasEqConstraint(qc, "counter_id"))
+    return RowCount() / 100;
+  return RowCount();
+}
+
+}  // namespace trace_processor
+}  // namespace perfetto
diff --git a/src/trace_processor/counter_values_table.h b/src/trace_processor/counter_values_table.h
new file mode 100644
index 0000000..c6581f2
--- /dev/null
+++ b/src/trace_processor/counter_values_table.h
@@ -0,0 +1,46 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef SRC_TRACE_PROCESSOR_COUNTER_VALUES_TABLE_H_
+#define SRC_TRACE_PROCESSOR_COUNTER_VALUES_TABLE_H_
+
+#include "src/trace_processor/storage_table.h"
+
+namespace perfetto {
+namespace trace_processor {
+
+// The implementation of the SQLite table containing the values of counters
+// in the trace.
+class CounterValuesTable : public StorageTable {
+ public:
+  static void RegisterTable(sqlite3* db, const TraceStorage* storage);
+
+  CounterValuesTable(sqlite3*, const TraceStorage*);
+
+  // StorageTable implementation.
+  StorageSchema CreateStorageSchema() override;
+  uint32_t RowCount() override;
+  int BestIndex(const QueryConstraints&, BestIndexInfo*) override;
+
+ private:
+  uint32_t EstimateCost(const QueryConstraints&);
+
+  const TraceStorage* const storage_;
+};
+}  // namespace trace_processor
+}  // namespace perfetto
+
+#endif  // SRC_TRACE_PROCESSOR_COUNTER_VALUES_TABLE_H_
diff --git a/src/trace_processor/counters_table.cc b/src/trace_processor/counters_table.cc
deleted file mode 100644
index efa7948..0000000
--- a/src/trace_processor/counters_table.cc
+++ /dev/null
@@ -1,155 +0,0 @@
-/*
- * Copyright (C) 2018 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include "src/trace_processor/counters_table.h"
-
-namespace perfetto {
-namespace trace_processor {
-
-CountersTable::CountersTable(sqlite3*, const TraceStorage* storage)
-    : storage_(storage) {
-  ref_types_.resize(RefType::kRefMax);
-  ref_types_[RefType::kRefNoRef] = "";
-  ref_types_[RefType::kRefUtid] = "utid";
-  ref_types_[RefType::kRefCpuId] = "cpu";
-  ref_types_[RefType::kRefIrq] = "irq";
-  ref_types_[RefType::kRefSoftIrq] = "softirq";
-  ref_types_[RefType::kRefUpid] = "upid";
-  ref_types_[RefType::kRefUtidLookupUpid] = "upid";
-}
-
-void CountersTable::RegisterTable(sqlite3* db, const TraceStorage* storage) {
-  Table::Register<CountersTable>(db, storage, "counters");
-}
-
-StorageSchema CountersTable::CreateStorageSchema() {
-  const auto& cs = storage_->counters();
-  return StorageSchema::Builder()
-      .AddColumn<IdColumn>("id", TableId::kCounters)
-      .AddOrderedNumericColumn("ts", &cs.timestamps())
-      .AddStringColumn("name", &cs.name_ids(), &storage_->string_pool())
-      .AddNumericColumn("value", &cs.values())
-      .AddColumn<RefColumn>("ref", &cs.refs(), &cs.types(), storage_)
-      .AddStringColumn("ref_type", &cs.types(), &ref_types_)
-      .AddNumericColumn("arg_set_id", &cs.arg_set_ids())
-      .Build({"name", "ts", "ref"});
-}
-
-uint32_t CountersTable::RowCount() {
-  return static_cast<uint32_t>(storage_->counters().counter_count());
-}
-
-int CountersTable::BestIndex(const QueryConstraints& qc, BestIndexInfo* info) {
-  info->estimated_cost =
-      static_cast<uint32_t>(storage_->counters().counter_count());
-
-  // Only the string columns are handled by SQLite
-  info->order_by_consumed = true;
-  size_t name_index = schema().ColumnIndexFromName("name");
-  size_t ref_type_index = schema().ColumnIndexFromName("ref_type");
-  for (size_t i = 0; i < qc.constraints().size(); i++) {
-    info->omit[i] =
-        qc.constraints()[i].iColumn != static_cast<int>(name_index) &&
-        qc.constraints()[i].iColumn != static_cast<int>(ref_type_index);
-  }
-
-  return SQLITE_OK;
-}
-
-CountersTable::RefColumn::RefColumn(std::string col_name,
-                                    const std::deque<int64_t>* refs,
-                                    const std::deque<RefType>* types,
-                                    const TraceStorage* storage)
-    : StorageColumn(col_name, false /* hidden */),
-      refs_(refs),
-      types_(types),
-      storage_(storage) {}
-
-void CountersTable::RefColumn::ReportResult(sqlite3_context* ctx,
-                                            uint32_t row) const {
-  auto ref = (*refs_)[row];
-  auto type = (*types_)[row];
-  if (type == RefType::kRefUtidLookupUpid) {
-    auto upid = storage_->GetThread(static_cast<uint32_t>(ref)).upid;
-    if (upid.has_value()) {
-      sqlite_utils::ReportSqliteResult(ctx, upid.value());
-    } else {
-      sqlite3_result_null(ctx);
-    }
-  } else {
-    sqlite_utils::ReportSqliteResult(ctx, ref);
-  }
-}
-
-CountersTable::RefColumn::Bounds CountersTable::RefColumn::BoundFilter(
-    int,
-    sqlite3_value*) const {
-  return Bounds{};
-}
-
-void CountersTable::RefColumn::Filter(int op,
-                                      sqlite3_value* value,
-                                      FilteredRowIndex* index) const {
-  bool op_is_null = sqlite_utils::IsOpIsNull(op);
-  auto predicate = sqlite_utils::CreateNumericPredicate<int64_t>(op, value);
-  index->FilterRows([this, &predicate, op_is_null](uint32_t row) {
-    auto ref = (*refs_)[row];
-    auto type = (*types_)[row];
-    if (type == RefType::kRefUtidLookupUpid) {
-      auto upid = storage_->GetThread(static_cast<uint32_t>(ref)).upid;
-      // Trying to filter null with any operation we currently handle
-      // should return false.
-      return upid.has_value() ? predicate(upid.value()) : op_is_null;
-    }
-    return predicate(ref);
-  });
-}
-
-CountersTable::RefColumn::Comparator CountersTable::RefColumn::Sort(
-    const QueryConstraints::OrderBy& ob) const {
-  if (ob.desc) {
-    return [this](uint32_t f, uint32_t s) { return -CompareRefsAsc(f, s); };
-  }
-  return [this](uint32_t f, uint32_t s) { return CompareRefsAsc(f, s); };
-}
-
-int CountersTable::RefColumn::CompareRefsAsc(uint32_t f, uint32_t s) const {
-  auto ref_f = (*refs_)[f];
-  auto ref_s = (*refs_)[s];
-
-  auto type_f = (*types_)[f];
-  auto type_s = (*types_)[s];
-
-  base::Optional<int64_t> val_f = ref_f;
-  base::Optional<int64_t> val_s = ref_s;
-  if (type_f == RefType::kRefUtidLookupUpid) {
-    val_f = storage_->GetThread(static_cast<uint32_t>(ref_f)).upid;
-  }
-  if (type_s == RefType::kRefUtidLookupUpid) {
-    val_s = storage_->GetThread(static_cast<uint32_t>(ref_s)).upid;
-  }
-
-  if (val_f.has_value() && val_s.has_value()) {
-    return sqlite_utils::CompareValuesAsc(val_f.value(), val_s.value());
-  } else if (!val_f.has_value()) {
-    return val_s.has_value() ? -1 : 0;
-  } else {
-    return 1;
-  }
-}
-
-}  // namespace trace_processor
-}  // namespace perfetto
diff --git a/src/trace_processor/counters_table_unittest.cc b/src/trace_processor/counters_table_unittest.cc
deleted file mode 100644
index 5a855fe..0000000
--- a/src/trace_processor/counters_table_unittest.cc
+++ /dev/null
@@ -1,240 +0,0 @@
-/*
- * Copyright (C) 2018 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include "src/trace_processor/counters_table.h"
-#include "src/trace_processor/event_tracker.h"
-#include "src/trace_processor/process_tracker.h"
-#include "src/trace_processor/scoped_db.h"
-#include "src/trace_processor/trace_processor_context.h"
-
-#include "gmock/gmock.h"
-#include "gtest/gtest.h"
-
-namespace perfetto {
-namespace trace_processor {
-namespace {
-
-class CountersTableUnittest : public ::testing::Test {
- public:
-  CountersTableUnittest() {
-    sqlite3* db = nullptr;
-    PERFETTO_CHECK(sqlite3_open(":memory:", &db) == SQLITE_OK);
-    db_.reset(db);
-
-    context_.storage.reset(new TraceStorage());
-    context_.event_tracker.reset(new EventTracker(&context_));
-    context_.process_tracker.reset(new ProcessTracker(&context_));
-
-    CountersTable::RegisterTable(db_.get(), context_.storage.get());
-  }
-
-  void PrepareValidStatement(const std::string& sql) {
-    int size = static_cast<int>(sql.size());
-    sqlite3_stmt* stmt;
-    ASSERT_EQ(sqlite3_prepare_v2(*db_, sql.c_str(), size, &stmt, nullptr),
-              SQLITE_OK);
-    stmt_.reset(stmt);
-  }
-
-  const char* GetColumnAsText(int colId) {
-    return reinterpret_cast<const char*>(sqlite3_column_text(*stmt_, colId));
-  }
-
-  ~CountersTableUnittest() override { context_.storage->ResetStorage(); }
-
- protected:
-  TraceProcessorContext context_;
-  ScopedDb db_;
-  ScopedStmt stmt_;
-};
-
-TEST_F(CountersTableUnittest, SelectWhereCpu) {
-  int64_t timestamp = 1000;
-  uint32_t freq = 3000;
-
-  context_.storage->mutable_counters()->AddCounter(
-      timestamp, 1, freq, 1 /* cpu */, RefType::kRefCpuId);
-  context_.storage->mutable_counters()->AddCounter(
-      timestamp + 1, 1, freq + 1000, 1 /* cpu */, RefType::kRefCpuId);
-  context_.storage->mutable_counters()->AddCounter(
-      timestamp + 2, 1, freq + 2000, 2 /* cpu */, RefType::kRefCpuId);
-
-  PrepareValidStatement(
-      "SELECT ts, "
-      "lead(ts) over (partition by name, ref order by ts) - ts as dur, "
-      "value "
-      "FROM counters where ref = 1");
-
-  ASSERT_EQ(sqlite3_step(*stmt_), SQLITE_ROW);
-  ASSERT_EQ(sqlite3_column_int(*stmt_, 0), timestamp);
-  ASSERT_EQ(sqlite3_column_int(*stmt_, 1), 1);
-  ASSERT_EQ(sqlite3_column_int(*stmt_, 2), freq);
-
-  ASSERT_EQ(sqlite3_step(*stmt_), SQLITE_ROW);
-  ASSERT_EQ(sqlite3_column_int(*stmt_, 0), timestamp + 1);
-  ASSERT_EQ(sqlite3_column_int(*stmt_, 1), 0);
-  ASSERT_EQ(sqlite3_column_int(*stmt_, 2), freq + 1000);
-
-  ASSERT_EQ(sqlite3_step(*stmt_), SQLITE_DONE);
-}
-
-TEST_F(CountersTableUnittest, GroupByFreq) {
-  int64_t timestamp = 1000;
-  uint32_t freq = 3000;
-  uint32_t name_id = 1;
-
-  context_.storage->mutable_counters()->AddCounter(
-      timestamp, name_id, freq, 1 /* cpu */, RefType::kRefCpuId);
-  context_.storage->mutable_counters()->AddCounter(
-      timestamp + 1, name_id, freq + 1000, 1 /* cpu */, RefType::kRefCpuId);
-  context_.storage->mutable_counters()->AddCounter(
-      timestamp + 3, name_id, freq, 1 /* cpu */, RefType::kRefCpuId);
-
-  PrepareValidStatement(
-      "SELECT value, sum(dur) as dur_sum "
-      "FROM ( "
-      "select value, "
-      "lead(ts) over (partition by name, ref order by ts) - ts as dur "
-      "from counters"
-      ") "
-      "where value > 0 "
-      "group by value "
-      "order by dur_sum desc");
-
-  ASSERT_EQ(sqlite3_step(*stmt_), SQLITE_ROW);
-  ASSERT_EQ(sqlite3_column_int(*stmt_, 0), freq + 1000);
-  ASSERT_EQ(sqlite3_column_int(*stmt_, 1), 2);
-
-  ASSERT_EQ(sqlite3_step(*stmt_), SQLITE_ROW);
-  ASSERT_EQ(sqlite3_column_int(*stmt_, 0), freq);
-  ASSERT_EQ(sqlite3_column_int(*stmt_, 1), 1);
-
-  ASSERT_EQ(sqlite3_step(*stmt_), SQLITE_DONE);
-}
-
-TEST_F(CountersTableUnittest, UtidLookupUpid) {
-  int64_t timestamp = 1000;
-  uint32_t value = 3000;
-  uint32_t name_id = 1;
-
-  uint32_t utid = context_.process_tracker->UpdateThread(timestamp, 1, 0);
-
-  context_.storage->mutable_counters()->AddCounter(
-      timestamp, name_id, value, utid, RefType::kRefUtidLookupUpid);
-
-  PrepareValidStatement("SELECT value, ref, ref_type FROM counters");
-
-  ASSERT_EQ(sqlite3_step(*stmt_), SQLITE_ROW);
-  ASSERT_EQ(sqlite3_column_int(*stmt_, 0), value);
-  ASSERT_EQ(sqlite3_column_type(*stmt_, 1), SQLITE_NULL);
-  ASSERT_STREQ(reinterpret_cast<const char*>(sqlite3_column_text(*stmt_, 2)),
-               "upid");
-
-  ASSERT_EQ(sqlite3_step(*stmt_), SQLITE_DONE);
-
-  // Simulate some other processes to avoid a situation where utid == upid so
-  // we cannot assert correctly below.
-  context_.process_tracker->UpdateProcess(4);
-  context_.process_tracker->UpdateProcess(10);
-  context_.process_tracker->UpdateProcess(11);
-
-  auto* thread = context_.storage->GetMutableThread(utid);
-  thread->upid = context_.process_tracker->UpdateProcess(1);
-
-  PrepareValidStatement("SELECT value, ref, ref_type FROM counters");
-
-  ASSERT_EQ(sqlite3_step(*stmt_), SQLITE_ROW);
-  ASSERT_EQ(sqlite3_column_int(*stmt_, 0), value);
-  ASSERT_EQ(sqlite3_column_int(*stmt_, 1), thread->upid.value());
-  ASSERT_STREQ(reinterpret_cast<const char*>(sqlite3_column_text(*stmt_, 2)),
-               "upid");
-
-  ASSERT_EQ(sqlite3_step(*stmt_), SQLITE_DONE);
-}
-
-TEST_F(CountersTableUnittest, UtidLookupUpidSort) {
-  int64_t timestamp = 1000;
-  uint32_t value = 3000;
-  uint32_t name_id = 1;
-
-  uint32_t utid_a = context_.process_tracker->UpdateThread(timestamp, 100, 0);
-  uint32_t utid_b = context_.process_tracker->UpdateThread(timestamp, 200, 0);
-
-  auto* thread_a = context_.storage->GetMutableThread(utid_a);
-  thread_a->upid = context_.process_tracker->UpdateProcess(100);
-
-  context_.storage->mutable_counters()->AddCounter(
-      timestamp, name_id, value, utid_a, RefType::kRefUtidLookupUpid);
-  context_.storage->mutable_counters()->AddCounter(
-      timestamp + 1, name_id, value, utid_b, RefType::kRefUtidLookupUpid);
-
-  PrepareValidStatement("SELECT ts, ref, ref_type FROM counters ORDER BY ref");
-
-  ASSERT_EQ(sqlite3_step(*stmt_), SQLITE_ROW);
-  ASSERT_EQ(sqlite3_column_int(*stmt_, 0), timestamp + 1);
-  ASSERT_EQ(sqlite3_column_type(*stmt_, 1), SQLITE_NULL);
-  ASSERT_STREQ(reinterpret_cast<const char*>(sqlite3_column_text(*stmt_, 2)),
-               "upid");
-
-  ASSERT_EQ(sqlite3_step(*stmt_), SQLITE_ROW);
-  ASSERT_EQ(sqlite3_column_int(*stmt_, 0), timestamp);
-  ASSERT_EQ(sqlite3_column_int(*stmt_, 1), thread_a->upid.value());
-  ASSERT_STREQ(reinterpret_cast<const char*>(sqlite3_column_text(*stmt_, 2)),
-               "upid");
-
-  ASSERT_EQ(sqlite3_step(*stmt_), SQLITE_DONE);
-}
-
-TEST_F(CountersTableUnittest, RefColumnComparator) {
-  int64_t timestamp = 1000;
-
-  UniquePid upid = context_.process_tracker->UpdateProcess(100);
-  // To ensure that upid and utid are not the same number
-  UniqueTid no_upid_tid =
-      context_.process_tracker->UpdateThread(timestamp, 101, 0);
-  UniqueTid utid = context_.process_tracker->UpdateThread(timestamp, 102, 0);
-  context_.storage->GetMutableThread(utid)->upid = upid;
-  ASSERT_NE(upid, utid);
-
-  uint32_t ctr_lookup_upid =
-      static_cast<uint32_t>(context_.storage->mutable_counters()->AddCounter(
-          timestamp, 0, 1 /* value */, utid, RefType::kRefUtidLookupUpid));
-
-  uint32_t ctr_upid =
-      static_cast<uint32_t>(context_.storage->mutable_counters()->AddCounter(
-          timestamp, 0, 1 /* value */, upid, RefType::kRefUpid));
-
-  uint32_t ctr_null_upid =
-      static_cast<uint32_t>(context_.storage->mutable_counters()->AddCounter(
-          timestamp, 0, 1 /* value */, no_upid_tid,
-          RefType::kRefUtidLookupUpid));
-
-  const auto& cs = context_.storage->counters();
-  CountersTable::RefColumn ref_column("ref", &cs.refs(), &cs.types(),
-                                      context_.storage.get());
-  auto comparator = ref_column.Sort(QueryConstraints::OrderBy());
-  // Lookup equality
-  ASSERT_EQ(comparator(ctr_lookup_upid, ctr_upid), 0);
-
-  // Null handling
-  ASSERT_EQ(comparator(ctr_upid, ctr_null_upid), 1);
-  ASSERT_EQ(comparator(ctr_null_upid, ctr_upid), -1);
-  ASSERT_EQ(comparator(ctr_null_upid, ctr_null_upid), 0);
-}
-
-}  // namespace
-}  // namespace trace_processor
-}  // namespace perfetto
diff --git a/src/trace_processor/event_tracker.cc b/src/trace_processor/event_tracker.cc
index 6d6b31a..473fc51 100644
--- a/src/trace_processor/event_tracker.cc
+++ b/src/trace_processor/event_tracker.cc
@@ -144,9 +144,12 @@
   }
   prev_timestamp_ = timestamp;
 
-  auto* counters = context_->storage->mutable_counters();
-  size_t idx = counters->AddCounter(timestamp, name_id, value, ref, ref_type);
-  return TraceStorage::CreateRowId(TableId::kCounters,
+  auto* definitions = context_->storage->mutable_counter_definitions();
+  auto counter_row = definitions->AddCounterDefinition(name_id, ref, ref_type);
+
+  auto* counter_values = context_->storage->mutable_counter_values();
+  size_t idx = counter_values->AddCounterValue(counter_row, timestamp, value);
+  return TraceStorage::CreateRowId(TableId::kCounterValues,
                                    static_cast<uint32_t>(idx));
 }
 
diff --git a/src/trace_processor/event_tracker_unittest.cc b/src/trace_processor/event_tracker_unittest.cc
index 1d1e926..8f15e96 100644
--- a/src/trace_processor/event_tracker_unittest.cc
+++ b/src/trace_processor/event_tracker_unittest.cc
@@ -120,15 +120,19 @@
   context.event_tracker->PushCounter(timestamp + 9, 1000, name_id, cpu,
                                      RefType::kRefCpuId);
 
-  ASSERT_EQ(context.storage->counters().counter_count(), 4ul);
-  ASSERT_EQ(context.storage->counters().timestamps().at(0), timestamp);
-  ASSERT_EQ(context.storage->counters().values().at(0), 1000);
+  ASSERT_EQ(context.storage->counter_definitions().size(), 1ul);
 
-  ASSERT_EQ(context.storage->counters().timestamps().at(1), timestamp + 1);
-  ASSERT_EQ(context.storage->counters().values().at(1), 4000);
+  ASSERT_EQ(context.storage->counter_values().size(), 4ul);
+  ASSERT_EQ(context.storage->counter_values().timestamps().at(0), timestamp);
+  ASSERT_EQ(context.storage->counter_values().values().at(0), 1000);
 
-  ASSERT_EQ(context.storage->counters().timestamps().at(2), timestamp + 3);
-  ASSERT_EQ(context.storage->counters().values().at(2), 5000);
+  ASSERT_EQ(context.storage->counter_values().timestamps().at(1),
+            timestamp + 1);
+  ASSERT_EQ(context.storage->counter_values().values().at(1), 4000);
+
+  ASSERT_EQ(context.storage->counter_values().timestamps().at(2),
+            timestamp + 3);
+  ASSERT_EQ(context.storage->counter_values().values().at(2), 5000);
 }
 
 }  // namespace
diff --git a/src/trace_processor/instants_table.cc b/src/trace_processor/instants_table.cc
index 39c093d..efdc15c 100644
--- a/src/trace_processor/instants_table.cc
+++ b/src/trace_processor/instants_table.cc
@@ -16,7 +16,7 @@
 
 #include "src/trace_processor/instants_table.h"
 
-#include "src/trace_processor/counters_table.h"
+#include "src/trace_processor/counter_definitions_table.h"
 
 namespace perfetto {
 namespace trace_processor {
@@ -44,8 +44,9 @@
       .AddOrderedNumericColumn("ts", &instants.timestamps())
       .AddStringColumn("name", &instants.name_ids(), &storage_->string_pool())
       .AddNumericColumn("value", &instants.values())
-      .AddColumn<CountersTable::RefColumn>("ref", &instants.refs(),
-                                           &instants.types(), storage_)
+      // TODO(lalitm): remove this hack.
+      .AddColumn<CounterDefinitionsTable::RefColumn>(
+          "ref", &instants.refs(), &instants.types(), storage_)
       .AddStringColumn("ref_type", &instants.types(), &ref_types_)
       .AddNumericColumn("arg_set_id", &instants.arg_set_ids())
       .Build({"name", "ts", "ref"});
diff --git a/src/trace_processor/storage_columns.cc b/src/trace_processor/storage_columns.cc
index 25cda66..b5ea94e 100644
--- a/src/trace_processor/storage_columns.cc
+++ b/src/trace_processor/storage_columns.cc
@@ -71,5 +71,9 @@
     : StorageColumn(std::move(column_name), false), table_id_(table_id) {}
 IdColumn::~IdColumn() = default;
 
+RowColumn::RowColumn(std::string column_name)
+    : StorageColumn(std::move(column_name), false) {}
+RowColumn::~RowColumn() = default;
+
 }  // namespace trace_processor
 }  // namespace perfetto
diff --git a/src/trace_processor/storage_columns.h b/src/trace_processor/storage_columns.h
index 7945bd7..7241ed4 100644
--- a/src/trace_processor/storage_columns.h
+++ b/src/trace_processor/storage_columns.h
@@ -342,6 +342,71 @@
   TableId table_id_;
 };
 
+// Column which is used to simply return the row index itself.
+class RowColumn final : public StorageColumn {
+ public:
+  RowColumn(std::string column_name);
+  virtual ~RowColumn() override;
+
+  void ReportResult(sqlite3_context* ctx, uint32_t row) const override {
+    sqlite_utils::ReportSqliteResult(ctx, row);
+  }
+
+  Bounds BoundFilter(int op, sqlite3_value* sqlite_val) const override {
+    Bounds bounds;
+
+    // Makes the below code much more readable.
+    using namespace sqlite_utils;
+
+    constexpr uint32_t kTMin = std::numeric_limits<uint32_t>::min();
+    constexpr uint32_t kTMax = std::numeric_limits<uint32_t>::max();
+
+    uint32_t min = kTMin;
+    uint32_t max = kTMax;
+    if (IsOpGe(op) || IsOpGt(op)) {
+      min = FindGtBound<uint32_t>(IsOpGe(op), sqlite_val);
+    } else if (IsOpLe(op) || IsOpLt(op)) {
+      max = FindLtBound<uint32_t>(IsOpLe(op), sqlite_val);
+    } else if (IsOpEq(op)) {
+      auto val = FindEqBound<uint32_t>(sqlite_val);
+      min = val;
+      max = val;
+    }
+
+    if (min == kTMin && max == kTMax)
+      return bounds;
+
+    bounds.min_idx = min;
+    bounds.max_idx = max + 1;
+    bounds.consumed = true;
+
+    return bounds;
+  }
+
+  void Filter(int op,
+              sqlite3_value* val,
+              FilteredRowIndex* index) const override {
+    index->FilterRows(sqlite_utils::CreateNumericPredicate<uint32_t>(op, val));
+  }
+
+  Comparator Sort(const QueryConstraints::OrderBy& ob) const override {
+    if (ob.desc) {
+      return [](uint32_t f, uint32_t s) {
+        return sqlite_utils::CompareValuesDesc(f, s);
+      };
+    }
+    return [](uint32_t f, uint32_t s) {
+      return sqlite_utils::CompareValuesAsc(f, s);
+    };
+  }
+
+  Table::ColumnType GetType() const override {
+    return Table::ColumnType::kUint;
+  }
+
+  bool IsNaturallyOrdered() const override { return true; }
+};
+
 }  // namespace trace_processor
 }  // namespace perfetto
 
diff --git a/src/trace_processor/storage_table.cc b/src/trace_processor/storage_table.cc
index 08e99af..c953e49 100644
--- a/src/trace_processor/storage_table.cc
+++ b/src/trace_processor/storage_table.cc
@@ -161,6 +161,16 @@
   return sorted_rows;
 }
 
+bool StorageTable::HasEqConstraint(const QueryConstraints& qc,
+                                   const std::string& col_name) {
+  size_t c_idx = schema().ColumnIndexFromName(col_name);
+  auto fn = [c_idx](const QueryConstraints::Constraint& c) {
+    return c.iColumn == static_cast<int>(c_idx) && sqlite_utils::IsOpEq(c.op);
+  };
+  const auto& cs = qc.constraints();
+  return std::find_if(cs.begin(), cs.end(), fn) != cs.end();
+}
+
 StorageTable::Cursor::Cursor(std::unique_ptr<RowIterator> iterator,
                              std::vector<std::unique_ptr<StorageColumn>>* cols)
     : iterator_(std::move(iterator)), columns_(std::move(cols)) {}
diff --git a/src/trace_processor/storage_table.h b/src/trace_processor/storage_table.h
index 510a059..987e005 100644
--- a/src/trace_processor/storage_table.h
+++ b/src/trace_processor/storage_table.h
@@ -62,6 +62,8 @@
  protected:
   const StorageSchema& schema() const { return schema_; }
 
+  bool HasEqConstraint(const QueryConstraints&, const std::string& col_name);
+
  private:
   // Creates a row iterator which is optimized for a generic storage schema
   // (i.e. it does not make assumptions about values of columns).
diff --git a/src/trace_processor/trace_processor_impl.cc b/src/trace_processor/trace_processor_impl.cc
index 80be94c..e4fb706 100644
--- a/src/trace_processor/trace_processor_impl.cc
+++ b/src/trace_processor/trace_processor_impl.cc
@@ -26,7 +26,8 @@
 #include "src/trace_processor/args_table.h"
 #include "src/trace_processor/args_tracker.h"
 #include "src/trace_processor/clock_tracker.h"
-#include "src/trace_processor/counters_table.h"
+#include "src/trace_processor/counter_definitions_table.h"
+#include "src/trace_processor/counter_values_table.h"
 #include "src/trace_processor/event_tracker.h"
 #include "src/trace_processor/instants_table.h"
 #include "src/trace_processor/process_table.h"
@@ -114,6 +115,19 @@
   }
 }
 
+void CreateBuiltinViews(sqlite3* db) {
+  char* error = nullptr;
+  sqlite3_exec(db,
+               "CREATE VIEW counters AS "
+               "SELECT * FROM counter_values "
+               "INNER JOIN counter_definitions USING(counter_id);",
+               0, 0, &error);
+  if (error) {
+    PERFETTO_ELOG("Error initializing: %s", error);
+    sqlite3_free(error);
+  }
+}
+
 bool IsPrefix(const std::string& a, const std::string& b) {
   return a.size() <= b.size() && b.substr(0, a.size()) == a;
 }
@@ -144,6 +158,7 @@
   PERFETTO_CHECK(sqlite3_open(":memory:", &db) == SQLITE_OK);
   InitializeSqlite(db);
   CreateBuiltinTables(db);
+  CreateBuiltinViews(db);
   db_.reset(std::move(db));
 
   context_.storage.reset(new TraceStorage());
@@ -163,7 +178,8 @@
   SqlStatsTable::RegisterTable(*db_, context_.storage.get());
   StringTable::RegisterTable(*db_, context_.storage.get());
   ThreadTable::RegisterTable(*db_, context_.storage.get());
-  CountersTable::RegisterTable(*db_, context_.storage.get());
+  CounterDefinitionsTable::RegisterTable(*db_, context_.storage.get());
+  CounterValuesTable::RegisterTable(*db_, context_.storage.get());
   SpanJoinOperatorTable::RegisterTable(*db_, context_.storage.get());
   WindowOperatorTable::RegisterTable(*db_, context_.storage.get());
   InstantsTable::RegisterTable(*db_, context_.storage.get());
diff --git a/src/trace_processor/trace_storage.cc b/src/trace_processor/trace_storage.cc
index 3259cb0..98e56bb 100644
--- a/src/trace_processor/trace_storage.cc
+++ b/src/trace_processor/trace_storage.cc
@@ -92,8 +92,8 @@
   int64_t end_ns = std::numeric_limits<int64_t>::min();
   MaybeUpdateMinMax(slices_.start_ns().begin(), slices_.start_ns().end(),
                     &start_ns, &end_ns);
-  MaybeUpdateMinMax(counters_.timestamps().begin(),
-                    counters_.timestamps().end(), &start_ns, &end_ns);
+  MaybeUpdateMinMax(counter_values_.timestamps().begin(),
+                    counter_values_.timestamps().end(), &start_ns, &end_ns);
   MaybeUpdateMinMax(instants_.timestamps().begin(),
                     instants_.timestamps().end(), &start_ns, &end_ns);
   MaybeUpdateMinMax(nestable_slices_.start_ns().begin(),
diff --git a/src/trace_processor/trace_storage.h b/src/trace_processor/trace_storage.h
index ca942db..d30fc14 100644
--- a/src/trace_processor/trace_storage.h
+++ b/src/trace_processor/trace_storage.h
@@ -52,7 +52,7 @@
 enum TableId : uint8_t {
   // Intentionally don't have TableId == 0 so that RowId == 0 can refer to an
   // invalid row id.
-  kCounters = 1,
+  kCounterValues = 1,
   kRawEvents = 2,
   kInstants = 3,
   kSched = 4,
@@ -331,44 +331,78 @@
     std::deque<int64_t> parent_stack_ids_;
   };
 
-  class Counters {
+  class CounterDefinitions {
    public:
-    inline size_t AddCounter(int64_t timestamp,
-                             StringId name_id,
-                             double value,
-                             int64_t ref,
-                             RefType type) {
-      timestamps_.emplace_back(timestamp);
+    using Id = uint32_t;
+
+    inline Id AddCounterDefinition(StringId name_id,
+                                   int64_t ref,
+                                   RefType type) {
+      base::Hash hash;
+      hash.Update(name_id);
+      hash.Update(ref);
+      hash.Update(type);
+
+      // TODO(lalitm): this is a perf bottleneck and likely we can do something
+      // quite a bit better here.
+      uint64_t digest = hash.digest();
+      auto it = hash_to_row_idx_.find(digest);
+      if (it != hash_to_row_idx_.end())
+        return it->second;
+
       name_ids_.emplace_back(name_id);
-      values_.emplace_back(value);
       refs_.emplace_back(ref);
       types_.emplace_back(type);
-      arg_set_ids_.emplace_back(kInvalidArgSetId);
-      return counter_count() - 1;
+      hash_to_row_idx_.emplace(digest, size() - 1);
+      return size() - 1;
     }
 
-    void set_arg_set_id(uint32_t row, ArgSetId id) { arg_set_ids_[row] = id; }
-
-    size_t counter_count() const { return timestamps_.size(); }
-
-    const std::deque<int64_t>& timestamps() const { return timestamps_; }
+    uint32_t size() const { return static_cast<uint32_t>(name_ids_.size()); }
 
     const std::deque<StringId>& name_ids() const { return name_ids_; }
 
-    const std::deque<double>& values() const { return values_; }
-
     const std::deque<int64_t>& refs() const { return refs_; }
 
     const std::deque<RefType>& types() const { return types_; }
 
+   private:
+    std::deque<StringId> name_ids_;
+    std::deque<int64_t> refs_;
+    std::deque<RefType> types_;
+
+    std::unordered_map<uint64_t, uint32_t> hash_to_row_idx_;
+  };
+
+  class CounterValues {
+   public:
+    inline uint32_t AddCounterValue(CounterDefinitions::Id counter_id,
+                                    int64_t timestamp,
+                                    double value) {
+      counter_ids_.emplace_back(counter_id);
+      timestamps_.emplace_back(timestamp);
+      values_.emplace_back(value);
+      arg_set_ids_.emplace_back(kInvalidArgSetId);
+      return size() - 1;
+    }
+
+    void set_arg_set_id(uint32_t row, ArgSetId id) { arg_set_ids_[row] = id; }
+
+    uint32_t size() const { return static_cast<uint32_t>(counter_ids_.size()); }
+
+    const std::deque<CounterDefinitions::Id>& counter_ids() const {
+      return counter_ids_;
+    }
+
+    const std::deque<int64_t>& timestamps() const { return timestamps_; }
+
+    const std::deque<double>& values() const { return values_; }
+
     const std::deque<ArgSetId>& arg_set_ids() const { return arg_set_ids_; }
 
    private:
+    std::deque<CounterDefinitions::Id> counter_ids_;
     std::deque<int64_t> timestamps_;
-    std::deque<StringId> name_ids_;
     std::deque<double> values_;
-    std::deque<int64_t> refs_;
-    std::deque<RefType> types_;
     std::deque<ArgSetId> arg_set_ids_;
   };
 
@@ -590,8 +624,15 @@
   const NestableSlices& nestable_slices() const { return nestable_slices_; }
   NestableSlices* mutable_nestable_slices() { return &nestable_slices_; }
 
-  const Counters& counters() const { return counters_; }
-  Counters* mutable_counters() { return &counters_; }
+  const CounterDefinitions& counter_definitions() const {
+    return counter_definitions_;
+  }
+  CounterDefinitions* mutable_counter_definitions() {
+    return &counter_definitions_;
+  }
+
+  const CounterValues& counter_values() const { return counter_values_; }
+  CounterValues* mutable_counter_values() { return &counter_values_; }
 
   const SqlStats& sql_stats() const { return sql_stats_; }
   SqlStats* mutable_sql_stats() { return &sql_stats_; }
@@ -660,9 +701,12 @@
   // Slices coming from userspace events (e.g. Chromium TRACE_EVENT macros).
   NestableSlices nestable_slices_;
 
-  // Counter events from the trace. This includes CPU frequency events as well
-  // systrace trace_marker counter events.
-  Counters counters_;
+  // The type of counters in the trace. Can be thought of the the "metadata".
+  CounterDefinitions counter_definitions_;
+
+  // The values from the Counter events from the trace. This includes CPU
+  // frequency events as well systrace trace_marker counter events.
+  CounterValues counter_values_;
 
   SqlStats sql_stats_;
 
diff --git a/test/trace_processor/counters_group_by_freq.py b/test/trace_processor/counters_group_by_freq.py
new file mode 100644
index 0000000..d30f3e3
--- /dev/null
+++ b/test/trace_processor/counters_group_by_freq.py
@@ -0,0 +1,28 @@
+#!/usr/bin/python
+# Copyright (C) 2019 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from os import sys, path
+
+sys.path.append(path.dirname(path.abspath(__file__)))
+import synth_common
+
+trace = synth_common.create_trace()
+
+trace.add_ftrace_packet(cpu=0)
+trace.add_cpufreq(ts=1000, freq=3000, cpu=1)
+trace.add_cpufreq(ts=1001, freq=4000, cpu=1)
+trace.add_cpufreq(ts=1003, freq=3000, cpu=1)
+
+print(trace.trace.SerializeToString())
diff --git a/test/trace_processor/counters_group_by_freq.sql b/test/trace_processor/counters_group_by_freq.sql
new file mode 100644
index 0000000..528712e
--- /dev/null
+++ b/test/trace_processor/counters_group_by_freq.sql
@@ -0,0 +1,11 @@
+SELECT
+  value,
+  sum(dur) as dur_sum
+FROM (
+  SELECT value,
+  lead(ts) OVER (PARTITION BY name, ref ORDER BY ts) - ts AS dur
+  FROM counters
+)
+WHERE value > 0
+GROUP BY value
+ORDER BY dur_sum desc
diff --git a/test/trace_processor/counters_group_by_freq_counters_group_by_freq.out b/test/trace_processor/counters_group_by_freq_counters_group_by_freq.out
new file mode 100644
index 0000000..bf3ddf4
--- /dev/null
+++ b/test/trace_processor/counters_group_by_freq_counters_group_by_freq.out
@@ -0,0 +1,3 @@
+"value","dur_sum"
+4000.000000,2
+3000.000000,1
diff --git a/test/trace_processor/counters_order_ref.py b/test/trace_processor/counters_order_ref.py
new file mode 100644
index 0000000..4cfc2b0
--- /dev/null
+++ b/test/trace_processor/counters_order_ref.py
@@ -0,0 +1,35 @@
+#!/usr/bin/python
+# Copyright (C) 2019 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from os import sys, path
+
+sys.path.append(path.dirname(path.abspath(__file__)))
+import synth_common
+
+trace = synth_common.create_trace()
+trace.add_process_tree_packet()
+trace.add_process(pid=1, ppid=0, cmdline="init")
+trace.add_process(pid=2, ppid=1, cmdline="two_thread_process")
+trace.add_process(pid=4, ppid=1, cmdline="single_thread_process")
+trace.add_thread(tid=3, tgid=2, cmdline="two_thread_process")
+
+trace.add_ftrace_packet(cpu=0)
+trace.add_rss_stat(ts=1000, tid=4, member=0, size=200)
+trace.add_rss_stat(ts=1005, tid=3, member=0, size=200)
+trace.add_rss_stat(ts=1010, tid=5, member=0, size=100)
+
+trace.add_oom_score_update(ts=1000, oom_score_adj=1000, pid=2)
+
+print(trace.trace.SerializeToString())
diff --git a/test/trace_processor/counters_order_ref.sql b/test/trace_processor/counters_order_ref.sql
new file mode 100644
index 0000000..bdf1ab0
--- /dev/null
+++ b/test/trace_processor/counters_order_ref.sql
@@ -0,0 +1,3 @@
+SELECT ts, ref, ref_type
+FROM counters
+ORDER BY ref
diff --git a/test/trace_processor/counters_order_ref_counters_order_ref.out b/test/trace_processor/counters_order_ref_counters_order_ref.out
new file mode 100644
index 0000000..b54d67a
--- /dev/null
+++ b/test/trace_processor/counters_order_ref_counters_order_ref.out
@@ -0,0 +1,5 @@
+"ts","ref","ref_type"
+1010,"[NULL]","upid"
+1000,2,"upid"
+1005,2,"upid"
+1000,3,"upid"
diff --git a/test/trace_processor/counters_where_cpu.py b/test/trace_processor/counters_where_cpu.py
new file mode 100644
index 0000000..0089289
--- /dev/null
+++ b/test/trace_processor/counters_where_cpu.py
@@ -0,0 +1,28 @@
+#!/usr/bin/python
+# Copyright (C) 2019 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from os import sys, path
+
+sys.path.append(path.dirname(path.abspath(__file__)))
+import synth_common
+
+trace = synth_common.create_trace()
+
+trace.add_ftrace_packet(cpu=0)
+trace.add_cpufreq(ts=1000, freq=3000, cpu=1)
+trace.add_cpufreq(ts=1001, freq=4000, cpu=1)
+trace.add_cpufreq(ts=1002, freq=5000, cpu=2)
+
+print(trace.trace.SerializeToString())
diff --git a/test/trace_processor/counters_where_cpu.sql b/test/trace_processor/counters_where_cpu.sql
new file mode 100644
index 0000000..e12fa4c
--- /dev/null
+++ b/test/trace_processor/counters_where_cpu.sql
@@ -0,0 +1,6 @@
+SELECT
+  ts,
+  lead(ts, 1, ts) OVER (PARTITION BY name ORDER BY ts) - ts AS dur,
+  value
+FROM counters
+WHERE ref = 1;
diff --git a/test/trace_processor/counters_where_cpu_counters_where_cpu.out b/test/trace_processor/counters_where_cpu_counters_where_cpu.out
new file mode 100644
index 0000000..ac7857f
--- /dev/null
+++ b/test/trace_processor/counters_where_cpu_counters_where_cpu.out
@@ -0,0 +1,3 @@
+"ts","dur","value"
+1000,1,3000.000000
+1001,0,4000.000000
diff --git a/test/trace_processor/index b/test/trace_processor/index
index 05ff6be..0f8cc2d 100644
--- a/test/trace_processor/index
+++ b/test/trace_processor/index
@@ -66,12 +66,17 @@
 
 # The below tests check the lower level layers of the trace processor (i.e.
 # fitering and printing code).
-# Filtering
+# Sched table
 synth_1.py filter_sched.sql synth_1_filter_sched.out
-synth_1.py filter_counters.sql synth_1_filter_counters.out
-../data/android_sched_and_ps.pb b119301023.sql android_sched_and_ps_b119301023.out
 ../data/android_sched_and_ps.pb b119496959.sql android_sched_and_ps_b119496959.out
+../data/android_sched_and_ps.pb b119301023.sql android_sched_and_ps_b119301023.out
+
+# Counters table
+synth_1.py filter_counters.sql synth_1_filter_counters.out
 ../data/memory_counters.pb b120278869_neg_ts_end.sql memory_counters_b120278869_neg_ts_end.out
+counters_where_cpu.py counters_where_cpu.sql counters_where_cpu_counters_where_cpu.out
+counters_group_by_freq.py counters_group_by_freq.sql counters_group_by_freq_counters_group_by_freq.out
+counters_order_ref.py counters_order_ref.sql counters_order_ref_counters_order_ref.out
 
 # Null printing
 synth_1.py nulls.sql nulls.out
diff --git a/test/trace_processor/mm_event.sql b/test/trace_processor/mm_event.sql
index 2a17587..187d0ff 100644
--- a/test/trace_processor/mm_event.sql
+++ b/test/trace_processor/mm_event.sql
@@ -1,4 +1,5 @@
-select *
+select id, ts, name, value, ref, ref_type, arg_set_id
 from counters
 where name like 'mem.mm.%'
+order by ts
 limit 40
diff --git a/test/trace_processor/oom_score_poll.sql b/test/trace_processor/oom_score_poll.sql
index 9abd98f..c572ac6 100644
--- a/test/trace_processor/oom_score_poll.sql
+++ b/test/trace_processor/oom_score_poll.sql
@@ -1,4 +1,5 @@
 select ts, name, value, ref
 from counters
 where name = "oom_score_adj"
+order by ts
 limit 20
diff --git a/test/trace_processor/synth_1.py b/test/trace_processor/synth_1.py
index 0b35455..b42db14 100644
--- a/test/trace_processor/synth_1.py
+++ b/test/trace_processor/synth_1.py
@@ -25,13 +25,13 @@
 trace.add_process(4, 1, "single_thread_process")
 trace.add_thread(3, 2, "two_thread_process")
 
-trace.add_ftrace_packet(0)
+trace.add_ftrace_packet(cpu=0)
 trace.add_sched(ts=1, prev_pid=1, next_pid=3)
 trace.add_cpufreq(ts=50, freq=50, cpu=0)
 trace.add_sched(ts=100, prev_pid=3, next_pid=2)
 trace.add_sched(ts=115, prev_pid=2, next_pid=3)
 
-trace.add_ftrace_packet(1)
+trace.add_ftrace_packet(cpu=1)
 trace.add_sched(ts=50, prev_pid=4, next_pid=1)
 trace.add_sched(ts=120, prev_pid=1, next_pid=2)
 trace.add_sched(ts=170, prev_pid=2, next_pid=0)
diff --git a/ui/src/controller/trace_controller.ts b/ui/src/controller/trace_controller.ts
index 944f66d..9dfff81 100644
--- a/ui/src/controller/trace_controller.ts
+++ b/ui/src/controller/trace_controller.ts
@@ -279,7 +279,7 @@
 
     const counters = await engine.query(`
       select name, ref, ref_type, count(ref_type)
-      from counters
+      from counter_definitions
       where ref is not null
       group by name, ref, ref_type
       order by ref_type desc