-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Re-land: Add callgrind collection to Timer #44717 #45586
Conversation
…imer" This reverts commit 51d0ae9.
💊 CI failures summary and remediationsAs of commit 6e9ad61 (more details on the Dr. CI page): ✅ None of the CI failures appear to be your fault 💚
🚧 1 ongoing upstream failure:These were probably caused by upstream breakages that are not fixed yet:
This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group. This comment has been revised 1 time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@robieta has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Huh, did we miss this last PR because slow tests aren't run by default? @seemethere do target specifiers work yet? O/W you can just push a synthetic PR to |
Want to bring up an issue with this PR: Should we ask for pre-built binary from distro, instead of having it as a submodule? |
cc @malfet we should just go back to vendoring the headers directly |
@ezyang we can totally do that, or simply create a mirror on github... |
Actually I tried creating a mirror on GitHub just to reroute through its CDN, but somehow I end up with VCS detection error. |
I need to do a follow up PR to address a couple papercuts anyway, which I was going to do today. Do we have a preference between vendoring the headers vs. a mirror repo? |
Header vendor seems much better to me |
Following PR should embed the headers: #45914 |
Test plan: The unit test has been softened to be less platform sensitive.