Skip to content

[Timer] Count number of executed instructions on macOS #2350

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

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Jan 18, 2021

In an effort to get performance timers for the Swift parser that are as accurate as possible, I found this PR by Graydon (swiftlang/swift#18658) from a couple years ago, adding a performance metric that counts the number of executed instructions on macOS. According to Graydon (and my intuition), this number is a lot less noisy than the time values.

With this PR, I'm trying to taking the measurements a little bit further by recording them with every LLVM Timer. This way, we get the measurement for free in every place that LLVM Timers are used (which happens to include measurements for the Swift parser, my original goal).

The code is mostly copy-based from either Graydon's PR or other places in Timer.h/cpp. I would appreciate a thorough code review, in particular regarding the implications of my changes to CMakeList.txt.

Also: I'm not sure, if swift/main is the correct branch to target, please let me know if I should target a different branch.

This supersedes #2270 which targeted apple/main.

@ahoppen
Copy link
Member Author

ahoppen commented Jan 18, 2021

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Jan 20, 2021

@swift-ci Please test

@ahoppen ahoppen force-pushed the instruction-counter-stable branch from 4415993 to cfe48c2 Compare January 22, 2021 13:55
@ahoppen
Copy link
Member Author

ahoppen commented Jan 22, 2021

@swift-ci Please test

In addition to wall time etc. this should allow us to get less noisy
values for time measurements.
@ahoppen ahoppen force-pushed the instruction-counter-stable branch from cfe48c2 to c1bc52c Compare January 22, 2021 16:52
@ahoppen
Copy link
Member Author

ahoppen commented Jan 22, 2021

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Jan 29, 2021

@swift-ci Please test linux platform

@ahoppen
Copy link
Member Author

ahoppen commented Jan 30, 2021

Linux failure seems unrelated and bot is passing again. Let’s try again.

@swift-ci Please test linux platform

@ahoppen
Copy link
Member Author

ahoppen commented Feb 2, 2021

@swift-ci Please test linux platform

@ahoppen
Copy link
Member Author

ahoppen commented Feb 4, 2021

Merging this into upstream LLVM now: https://reviews.llvm.org/D96049

@ahoppen ahoppen closed this Feb 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant