Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Collect timing data per unit for each phase #4512

Merged
merged 7 commits into from
Nov 12, 2024

Conversation

samestep
Copy link
Contributor

This PR adds a --dump-timings flag to the compile subcommand (similar to the existing --dump-mem-usage flag), which collects timing data per compilation unit for each compilation phase. For example, on my 2020 M1 MacBook:

$ bazel build -c opt //toolchain
$ bazel-bin/toolchain/install/run_carbon compile --phase=lower --dump-timings examples/sieve.carbon | tail
...
---
filename:        'examples/sieve.carbon'
nanoseconds:
  lex:             30792
  parse:           25458
  check:           226625
  lower:           1136958
  Total:           1419833
...

Most of the changes are pretty straightforward. There were a couple I wasn't sure about though; let me know if I should change:

  • new Timings class in its own file, pretty similar to the existing MemUsage class
  • added a timings_ field to the CompilationUnit class
  • added a timings field to the Check::Unit struct
  • renamed CheckParseTree function to CheckParseTreeInner for ease of timing with early return

Copy link
Contributor

@jonmeow jonmeow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! This looks good, just a few small style things for Carbon's codebase.

toolchain/check/check.h Outdated Show resolved Hide resolved
toolchain/driver/compile_subcommand.cpp Outdated Show resolved Hide resolved
toolchain/driver/compile_subcommand.cpp Outdated Show resolved Hide resolved
toolchain/check/check.cpp Outdated Show resolved Hide resolved
toolchain/base/timings.h Outdated Show resolved Hide resolved
@samestep
Copy link
Contributor Author

Done! Interesting that the style here for an optional pointer is the opposite of, e.g., the style in rust-analyzer.

All look good?

Copy link
Contributor

@jonmeow jonmeow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

toolchain/base/timings.h Outdated Show resolved Hide resolved
auto-merge was automatically disabled November 12, 2024 18:51

Head branch was pushed to by a user without write access

@jonmeow jonmeow added this pull request to the merge queue Nov 12, 2024
Merged via the queue into carbon-language:trunk with commit e0e3055 Nov 12, 2024
8 checks passed
@samestep samestep deleted the timings branch November 12, 2024 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants