dmd icon indicating copy to clipboard operation
dmd copied to clipboard

Add --ftime-trace to dmd

Open dkorpel opened this issue 1 year ago • 2 comments
trafficstars

Reboot of https://github.com/dlang/dmd/pull/13965

This feature has been in LDC for a while now with success, and inclusion into dmd has been pre-approved a long time ago by Walter, but the PR has stalled since then. There's improvements possible with choosing better zones and refactoring the code, but for now I focused on making it closely match LDC's current implementation.

For this PR, I still need to include the timetrace2txt tool, and test it a bit better.

dkorpel avatar Apr 07 '24 19:04 dkorpel

Thanks for your pull request and interest in making D better, @dkorpel! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#16363"

dlang-bot avatar Apr 07 '24 19:04 dlang-bot

I've tested what the overhead is of normal compilation without -ftime-trace on a copy of Phobos.

hyperfine "dmds0 -i -c mystd/package.d" "dmds1 -i -c mystd/package.d"
Benchmark 1: dmds0 -i -c mystd/package.d
  Time (mean ± σ):      1.540 s ±  0.019 s    [User: 1.213 s, System: 0.316 s]
  Range (min … max):    1.520 s …  1.576 s    10 runs
 
Benchmark 2: dmds1 -i -c mystd/package.d
  Time (mean ± σ):      1.553 s ±  0.011 s    [User: 1.206 s, System: 0.335 s]
  Range (min … max):    1.538 s …  1.572 s    10 runs
 
Summary
  dmds0 -i -c mystd/package.d ran
    1.01 ± 0.01 times faster than dmds1 -i -c mystd/package.d

(dmds0 = master branch, dmds1 = ftime-trace branch)

So ~1% overhead, which is worth it when you see how much build time this feature can save.

dkorpel avatar Apr 19 '24 15:04 dkorpel

@ibuclaw @JohanEngelen anything else blocking this PR?

RazvanN7 avatar Jul 22 '24 08:07 RazvanN7

@ibuclaw @JohanEngelen anything else blocking this PR?

I have not reviewed in detail this time, but I trust it is OK. A great addition to the frontend!

JohanEngelen avatar Jul 22 '24 17:07 JohanEngelen