pw_metric: Add size report
This adds a size report for pw_metric, not including the RPC service.
The report shows the impact of having a few metrics, of dumping, and of
adding more metrics.
At this time, the flash usage on Cortex-M4 is unexpectedly large due to
the creation of large static constructors. This will require followup
CLs to address.
This CL also moves a few inlined methods from the .h file into the .cc
to reduce code size.
Change-Id: Ie0cb041a6e1e5e77c921e9078adea2eaabccdd43
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/16300
Commit-Queue: Keir Mierle <keir@google.com>
Reviewed-by: Alexei Frolov <frolv@google.com>
diff --git a/pw_metric/BUILD.gn b/pw_metric/BUILD.gn
index 9a0ff2b..8829e8d 100644
--- a/pw_metric/BUILD.gn
+++ b/pw_metric/BUILD.gn
@@ -15,6 +15,7 @@
# gn-format disable
import("//build_overrides/pigweed.gni")
+import("$dir_pw_bloat/bloat.gni")
import("$dir_pw_build/target_types.gni")
import("$dir_pw_docgen/docs.gni")
import("$dir_pw_protobuf_compiler/proto.gni")
@@ -86,10 +87,6 @@
################################################################################
-pw_doc_group("docs") {
- sources = [ "docs.rst" ]
-}
-
pw_test_group("tests") {
tests = [
":metric_test",
@@ -109,3 +106,34 @@
sources = [ "global_test.cc" ]
deps = [ ":global" ]
}
+
+pw_size_report("metric_size_report") {
+ title = "Typical pw_metric use (no RPC service)"
+
+ # To see all the symbols, uncomment the following:
+ # Note: The size report RST table won't be generated when full_report = true.
+ #full_report = true
+
+ binaries = [
+ {
+ target = "size_report:one_metric"
+ base = "size_report:base"
+ label = "1 metric and 1 group no dump or export"
+ },
+ {
+ target = "size_report:dump"
+ base = "size_report:base"
+ label = "(+) dump group and metrics to log"
+ },
+ {
+ target = "size_report:more_metrics"
+ base = "size_report:dump"
+ label = "(+) 1 group (+) 4 metrics"
+ },
+ ]
+}
+
+pw_doc_group("docs") {
+ sources = [ "docs.rst" ]
+ report_deps = [ ":metric_size_report" ]
+}
diff --git a/pw_metric/docs.rst b/pw_metric/docs.rst
index 23ef778..7a736ce 100644
--- a/pw_metric/docs.rst
+++ b/pw_metric/docs.rst
@@ -711,6 +711,20 @@
pumping the metrics into the streaming response. This gives flow control to
the application.
+-----------
+Size report
+-----------
+The below size report shows the cost in code and memory for a few examples of
+metrics. This does not include the RPC service.
+
+.. include:: metric_size_report
+
+.. attention::
+
+ At time of writing, **the above sizes show an unexpectedly large flash
+ impact**. We are investigating why GCC is inserting large global static
+ constructors per group, when all the logic should be reused across objects.
+
----------------
Design tradeoffs
----------------
diff --git a/pw_metric/metric.cc b/pw_metric/metric.cc
index 9f0c990..5e99329 100644
--- a/pw_metric/metric.cc
+++ b/pw_metric/metric.cc
@@ -59,6 +59,31 @@
metrics.push_front(*this);
}
+float Metric::as_float() const {
+ PW_DCHECK(is_float());
+ return float_;
+}
+
+uint32_t Metric::as_int() const {
+ PW_DCHECK(is_int());
+ return uint_;
+}
+
+void Metric::Increment(uint32_t amount) {
+ PW_DCHECK(is_int());
+ uint_ += amount;
+}
+
+void Metric::SetInt(uint32_t value) {
+ PW_DCHECK(is_int());
+ uint_ = value;
+}
+
+void Metric::SetFloat(float value) {
+ PW_DCHECK(is_float());
+ float_ = value;
+}
+
void Metric::Dump(int level) {
Base64EncodedToken encoded_name(name());
const char* indent = Indent(level);
@@ -78,6 +103,12 @@
}
}
+Group::Group(Token name) : name_(name) {}
+
+Group::Group(Token name, IntrusiveList<Group>& groups) : name_(name) {
+ groups.push_front(*this);
+}
+
void Group::Dump(int level) {
Base64EncodedToken encoded_name(name());
const char* indent = Indent(level);
diff --git a/pw_metric/public/pw_metric/metric.h b/pw_metric/public/pw_metric/metric.h
index c217587..9c45ea0 100644
--- a/pw_metric/public/pw_metric/metric.h
+++ b/pw_metric/public/pw_metric/metric.h
@@ -46,15 +46,8 @@
bool is_float() const { return (name_and_type_ & kTypeMask) == kTypeFloat; }
bool is_int() const { return (name_and_type_ & kTypeMask) == kTypeInt; }
- float as_float() const {
- PW_DCHECK(is_float());
- return float_;
- }
-
- uint32_t as_int() const {
- PW_DCHECK(is_int());
- return uint_;
- }
+ float as_float() const;
+ uint32_t as_int() const;
// Dump a metric or metrics to logs. Level determines the indentation
// indent_level up to a maximum of 4. Example output:
@@ -83,20 +76,11 @@
// Hide mutation methods, and only offer write access through the specialized
// TypedMetric below. This makes it impossible to call metric.Increment() on
// a float metric at compile time.
- void Increment(uint32_t amount = 1) {
- PW_DCHECK(is_int());
- uint_ += amount;
- }
+ void Increment(uint32_t amount = 1);
- void SetInt(uint32_t value) {
- PW_DCHECK(is_int());
- uint_ = value;
- }
+ void SetInt(uint32_t value);
- void SetFloat(float value) {
- PW_DCHECK(is_float());
- float_ = value;
- }
+ void SetFloat(float value);
private:
// The name of this metric as a token; from PW_TOKENIZE_STRING("my_metric").
@@ -163,10 +147,8 @@
// Size: 16 bytes/128 bits - next, name, metrics, children.
class Group : public IntrusiveList<Group>::Item {
public:
- Group(Token name) : name_(name) {}
- Group(Token name, IntrusiveList<Group>& groups) : name_(name) {
- groups.push_front(*this);
- }
+ Group(Token name);
+ Group(Token name, IntrusiveList<Group>& groups);
Token name() const { return name_; }
diff --git a/pw_metric/size_report/BUILD b/pw_metric/size_report/BUILD
new file mode 100644
index 0000000..f7693a9
--- /dev/null
+++ b/pw_metric/size_report/BUILD
@@ -0,0 +1,65 @@
+# Copyright 2020 The Pigweed Authors
+#
+# 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
+#
+# https://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.
+
+load(
+ "//pw_build:pigweed.bzl",
+ "pw_cc_binary",
+)
+
+package(default_visibility = ["//visibility:public"])
+
+licenses(["notice"]) # Apache License 2.0
+
+pw_cc_binary(
+ name = "base",
+ srcs = ["base.cc"],
+ deps = [
+ "//pw_bloat:bloat_this_binary",
+ "//pw_log",
+ "//pw_assert",
+ ],
+)
+
+pw_cc_binary(
+ name = "one_metric",
+ srcs = ["one_metric.cc"],
+ deps = [
+ "//pw_bloat:bloat_this_binary",
+ "//pw_metric",
+ "//pw_log",
+ "//pw_assert",
+ ],
+)
+
+pw_cc_binary(
+ name = "dump",
+ srcs = ["dump.cc"],
+ deps = [
+ "//pw_bloat:bloat_this_binary",
+ "//pw_metric",
+ "//pw_log",
+ "//pw_assert",
+ ],
+)
+
+pw_cc_binary(
+ name = "more_metrics",
+ srcs = ["more_metrics.cc"],
+ deps = [
+ "//pw_bloat:bloat_this_binary",
+ "//pw_metric",
+ "//pw_log",
+ "//pw_assert",
+ ],
+)
diff --git a/pw_metric/size_report/BUILD.gn b/pw_metric/size_report/BUILD.gn
new file mode 100644
index 0000000..90d7ef0
--- /dev/null
+++ b/pw_metric/size_report/BUILD.gn
@@ -0,0 +1,56 @@
+# Copyright 2020 The Pigweed Authors
+#
+# 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
+#
+# https://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.
+
+# gn-format disable
+import("//build_overrides/pigweed.gni")
+
+import("$dir_pw_build/target_types.gni")
+pw_executable("base") {
+ sources = [ "base.cc" ]
+ deps = [
+ "$dir_pw_bloat:bloat_this_binary",
+ dir_pw_assert,
+ dir_pw_log,
+ ]
+}
+
+pw_executable("one_metric") {
+ sources = [ "one_metric.cc" ]
+ deps = [
+ "$dir_pw_bloat:bloat_this_binary",
+ "..",
+ dir_pw_assert,
+ dir_pw_log,
+ ]
+}
+
+pw_executable("dump") {
+ sources = [ "dump.cc" ]
+ deps = [
+ "$dir_pw_bloat:bloat_this_binary",
+ "..",
+ dir_pw_assert,
+ dir_pw_log,
+ ]
+}
+
+pw_executable("more_metrics") {
+ sources = [ "more_metrics.cc" ]
+ deps = [
+ "$dir_pw_bloat:bloat_this_binary",
+ "..",
+ dir_pw_assert,
+ dir_pw_log,
+ ]
+}
diff --git a/pw_metric/size_report/base.cc b/pw_metric/size_report/base.cc
new file mode 100644
index 0000000..8fb1abc
--- /dev/null
+++ b/pw_metric/size_report/base.cc
@@ -0,0 +1,31 @@
+// Copyright 2019 The Pigweed Authors
+//
+// 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
+//
+// https://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 "pw_assert/assert.h"
+#include "pw_bloat/bloat_this_binary.h"
+#include "pw_log/log.h"
+
+int volatile* unoptimizable;
+
+int main() {
+ pw::bloat::BloatThisBinary();
+
+ // Ensure we are paying the cost for log and assert.
+ PW_CHECK_INT_GE(unoptimizable, 0, "Ensure this CHECK logic stays");
+ PW_LOG_INFO("We care about optimizing: %d", *unoptimizable);
+ // This matches the log preventing optimizing the "m" metric in one_metric.cc.
+ PW_LOG_INFO("some_metric: %d", *unoptimizable);
+
+ return *unoptimizable;
+}
diff --git a/pw_metric/size_report/dump.cc b/pw_metric/size_report/dump.cc
new file mode 100644
index 0000000..b7ec068
--- /dev/null
+++ b/pw_metric/size_report/dump.cc
@@ -0,0 +1,45 @@
+// Copyright 2020 The Pigweed Authors
+//
+// 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
+//
+// https://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 "pw_assert/assert.h"
+#include "pw_bloat/bloat_this_binary.h"
+#include "pw_log/log.h"
+#include "pw_metric/metric.h"
+
+PW_METRIC_GROUP(group_foo, "some_group");
+PW_METRIC(group_foo, metric_x, "some_metric", 14u);
+
+int volatile* unoptimizable;
+
+int main() {
+ pw::bloat::BloatThisBinary();
+
+ if (*unoptimizable) {
+ metric_x.Increment();
+ }
+ metric_x.Increment();
+
+ // Ensure log and assert aren't optimized out.
+ PW_CHECK_INT_GE(unoptimizable, 0, "Ensure this CHECK logic stays");
+ PW_LOG_INFO("Ensure logs are pulled in: %d", *unoptimizable);
+
+ // Ensure metric_x isn't optimized out.
+ PW_LOG_INFO("metric_x: %d", static_cast<int>(metric_x.value()));
+
+ // Trigger pulling in the dump code. Dump twice to cover cost in more_metrics.
+ group_foo.Dump();
+ group_foo.Dump();
+
+ return *unoptimizable;
+}
diff --git a/pw_metric/size_report/more_metrics.cc b/pw_metric/size_report/more_metrics.cc
new file mode 100644
index 0000000..883a8ca
--- /dev/null
+++ b/pw_metric/size_report/more_metrics.cc
@@ -0,0 +1,65 @@
+// Copyright 2020 The Pigweed Authors
+//
+// 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
+//
+// https://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 "pw_assert/assert.h"
+#include "pw_bloat/bloat_this_binary.h"
+#include "pw_log/log.h"
+#include "pw_metric/metric.h"
+
+PW_METRIC_GROUP(group_bar, "bar");
+PW_METRIC(group_bar, metric_a, "a", 14u);
+PW_METRIC(group_bar, metric_b, "b", 14u);
+PW_METRIC(group_bar, metric_c, "c", 14u);
+
+PW_METRIC_GROUP(group_foo, "foo");
+PW_METRIC(group_foo, metric_x, "x", 14u);
+PW_METRIC(group_foo, metric_y, "y", 14u);
+
+int volatile* unoptimizable;
+
+int main() {
+ pw::bloat::BloatThisBinary();
+
+ if (*unoptimizable) {
+ // Bar
+ metric_a.Increment();
+ metric_b.Increment();
+ metric_c.Increment();
+
+ // Foo
+ metric_x.Increment();
+ metric_y.Increment();
+ }
+ // Bar
+ metric_a.Increment();
+ metric_b.Increment();
+ metric_c.Increment();
+
+ // Foo
+ metric_x.Increment();
+ metric_y.Increment();
+
+ // Ensure log and assert aren't optimized out.
+ PW_CHECK_INT_GE(unoptimizable, 0, "Ensure this CHECK logic stays");
+ PW_LOG_INFO("Ensure logs are pulled in: %d", *unoptimizable);
+
+ // Ensure metric_x isn't optimized out.
+ PW_LOG_INFO("metric_x: %d", static_cast<int>(metric_x.value()));
+
+ // Trigger pulling in the dump code.
+ group_foo.Dump();
+ group_bar.Dump();
+
+ return *unoptimizable;
+}
diff --git a/pw_metric/size_report/one_metric.cc b/pw_metric/size_report/one_metric.cc
new file mode 100644
index 0000000..4f08d07
--- /dev/null
+++ b/pw_metric/size_report/one_metric.cc
@@ -0,0 +1,42 @@
+// Copyright 2020 The Pigweed Authors
+//
+// 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
+//
+// https://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 "pw_assert/assert.h"
+#include "pw_bloat/bloat_this_binary.h"
+#include "pw_log/log.h"
+#include "pw_metric/metric.h"
+
+PW_METRIC_GROUP(group_foo, "foo");
+PW_METRIC(group_foo, metric_x, "x", 14u);
+
+int volatile* unoptimizable;
+
+int main() {
+ PW_METRIC_GROUP(inner, "some_group");
+ pw::bloat::BloatThisBinary();
+
+ if (*unoptimizable) {
+ metric_x.Increment();
+ }
+ metric_x.Increment();
+
+ // Ensure log and assert aren't optimized out.
+ PW_CHECK_INT_GE(unoptimizable, 0, "Ensure this CHECK logic stays");
+ PW_LOG_INFO("Ensure logs are pulled in: %d", *unoptimizable);
+
+ // Ensure metric_x isn't optimized out.
+ PW_LOG_INFO("metric_x: %d", static_cast<int>(metric_x.value()));
+
+ return *unoptimizable;
+}
