Add __torch_function__ benchmarks.#34645
Add __torch_function__ benchmarks.#34645hameerabbasi wants to merge 5 commits intopytorch:masterfrom
Conversation
|
Note: Do NOT merge this yet. It'll need updating after #34303 is merged. |
c639ed9 to
b78eaa0
Compare
💊 CircleCI build failures summary and remediationsAs of commit f5fcd1d (more details on the Dr. CI page):
🕵️ 4 new failures recognized by patternsThe following build failures do not appear to be due to upstream breakages:
|
rgommers
left a comment
There was a problem hiding this comment.
Still a bit too concise - the goal is really to make sure we get complete and consistent benchmarking results for future PRs that touch the dispatching code.
b78eaa0 to
042a084
Compare
|
The |
|
@ngoldbaum I'll let you review first! |
|
|
||
| * Run `bench.py`, and include the output in your result. | ||
| * For each case where `bench.py` shows a regression, run the commands described above, prefixing the output SVG filename (the input to the `-o` switch) with `base-` or `branch-` depending on the commit you are running the benchmark on. | ||
| * For each SVG, open it in the browser, take a screenshot and include it in your result. Also include a ZIP file with all SVGs thus produced included. |
There was a problem hiding this comment.
Couldn't this part be automated? Maybe rather than relying on the py-spy flame graph svg output it would make more sense to process the data from the "raw" output format to extract relevant information that can be copy/pasted into a PR discussion.
There was a problem hiding this comment.
Are there docs on that format somewhere?
There was a problem hiding this comment.
It’s mentioned in the output of py-spy record —help
There was a problem hiding this comment.
I mean the actual format that raw produces, is there a parser or some docs on it?
There was a problem hiding this comment.
No I don't think it's documented, py-spy's docs are pretty barebones. Looking at the py-spy source code it looks like it's writing the contents of the HashMap mapping stack frame names to counts to the output file, see:
There was a problem hiding this comment.
The py-spy raw output is in the 'folded stack' format used by flamegraph.pl (https://github.com/brendangregg/FlameGraph#2-fold-stacks) . The format is a text file with a stack trace /sample count per line, with each frame in the stack trace being delimited with a ';'. Fwiw for benchmarking, you can take a 'before' and 'after' version of these raw files and compute a differential flamegraph.
There was a problem hiding this comment.
I looked into this. It would introduce another dependency to rasterize the SVG, and another to diff the flame graph. I wonder if it's worth it.
There was a problem hiding this comment.
Not to mention auto checkout and auto builds -- Which is bad.
e0f1d95 to
f3629ee
Compare
f3629ee to
17dc58b
Compare
|
I'm happy to merge this when y'all are OK with it. Note that you should consider running the bench as part of our CI to make sure it doesn't bitrot. (Don't have to look at the numbers though) |
Where would I make these changes? And is it okay if it takes O(10 s) to run all of them, or should I reduce the number of reps via command line arguments? |
|
Reduce the number of reps. To add new scripts to CI look at |
d0de1c9 to
ffe35e5
Compare
ffe35e5 to
f5fcd1d
Compare
|
CI failures seem completely unrelated. |
facebook-github-bot
left a comment
There was a problem hiding this comment.
@ezyang is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
Looks like internal tests are failing -- Anything I can do to help fix this? |
|
they're fake, don't worry |
|
Non-ASCII characters broke tests: |
Summary: Re-land of pytorch#35530 and pytorch#34645 Pull Request resolved: pytorch#36138 Differential Revision: D20893770 Pulled By: ezyang fbshipit-source-id: 75ab688a086f5fb87412a853df5246c0c39704ca
Summary: Pull Request resolved: pytorch#34645 Differential Revision: D20653072 Pulled By: ezyang fbshipit-source-id: e7e363f8a1b84fc0c354586e266a695e4a2ea60e
Summary: Re-land of pytorch#35530 and pytorch#34645 Pull Request resolved: pytorch#36138 Differential Revision: D20893770 Pulled By: ezyang fbshipit-source-id: 75ab688a086f5fb87412a853df5246c0c39704ca
No description provided.