diff options
author | David 'Digit' Turner <digit@google.com> | 2022-05-12 16:18:16 (GMT) |
---|---|---|
committer | David 'Digit' Turner <digit+github@google.com> | 2023-09-25 10:50:22 (GMT) |
commit | 7289d5c2561f5789bec9c4f0114b73eb0e7476ee (patch) | |
tree | 3965985f173af3841303598329c1a830a955ff54 /src | |
parent | f64267fd947697649b45efeab04c241584b549cb (diff) | |
download | Ninja-7289d5c2561f5789bec9c4f0114b73eb0e7476ee.zip Ninja-7289d5c2561f5789bec9c4f0114b73eb0e7476ee.tar.gz Ninja-7289d5c2561f5789bec9c4f0114b73eb0e7476ee.tar.bz2 |
Fix .ninja parse time reported by `-d stats`.
Because Parser::Load() is called recursively during Ninja
manifest parsing, the call to METRIC_RECORD() in this function
used to over-count the total parsing time (for example, by a
factor of 2 for the Fuchsia build).
This fixes the problem by introducing a new RECORD_METRIC_IF()
macro, which only records anything if a given condition is true.
This ensures that metric collection only starts and stops with
the outer Parser::Load() call, and none of its recursive
sub-calls.
The effect on the output of `-d stats`is, for a Fuchsia build
plan where `ninja -d stats nothing` takes a bit more than 5s:
BEFORE:
metric count avg (us) total (ms)
.ninja parse 27304 372.6 10172.2
AFTER:
metric count avg (us) total (ms)
.ninja parse 1 4165297.0 4165.3
Note that |count| went to 1, since there is only one top-level
Parser::Load() operation in this build. It would be more if
dyndeps files were loaded, which does not happen in this
build plan.
Diffstat (limited to 'src')
-rw-r--r-- | src/metrics.h | 7 | ||||
-rw-r--r-- | src/parser.cc | 5 |
2 files changed, 11 insertions, 1 deletions
diff --git a/src/metrics.h b/src/metrics.h index c9ba236..937d905 100644 --- a/src/metrics.h +++ b/src/metrics.h @@ -85,6 +85,13 @@ struct Stopwatch { g_metrics ? g_metrics->NewMetric(name) : NULL; \ ScopedMetric metrics_h_scoped(metrics_h_metric); +/// A variant of METRIC_RECORD that doesn't record anything if |condition| +/// is false. +#define METRIC_RECORD_IF(name, condition) \ + static Metric* metrics_h_metric = \ + g_metrics ? g_metrics->NewMetric(name) : NULL; \ + ScopedMetric metrics_h_scoped((condition) ? metrics_h_metric : NULL); + extern Metrics* g_metrics; #endif // NINJA_METRICS_H_ diff --git a/src/parser.cc b/src/parser.cc index 5f303c5..139a347 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -20,7 +20,10 @@ using namespace std; bool Parser::Load(const string& filename, string* err, Lexer* parent) { - METRIC_RECORD(".ninja parse"); + // If |parent| is not NULL, metrics collection has been started by a parent + // Parser::Load() in our call stack. Do not start a new one here to avoid + // over-counting parsing times. + METRIC_RECORD_IF(".ninja parse", parent == NULL); string contents; string read_err; if (file_reader_->ReadFile(filename, &contents, &read_err) != |