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

Remove generation of graph plots from benchmark runs #2228

Merged
merged 12 commits into from
Apr 12, 2023

Conversation

agunapal
Copy link
Collaborator

@agunapal agunapal commented Apr 12, 2023

Description

I noticed that we generate plots during benchmark runs and these take a while
These plots are not being used anywhere

  • Introduced an optional parameter called generate_graphs with default value False

Fixes #(issue)

I believe this should speed up benchmark runs by at least 10-15%
Will update the exact numbers after a nightly run.

  • Reduced GPU benchmark time from 2 hour 27 min to 1 hour 40 min

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Feature/Issue validation/testing

Before

*Generating CSV output...
Saving benchmark results to /tmp
*Preparing graphs...
*Preparing Profile graphs...
Working with sampling rate of 100

Test suite execution complete.

After

*Generating CSV output...
Saving benchmark results to /tmp

Test suite execution complete.

Checklist:

  • Did you have fun?
  • Have you added tests that prove your fix is effective or that this feature works?
  • Has code been commented, particularly in hard-to-understand areas?
  • Have you made corresponding changes to the documentation?

@codecov
Copy link

codecov bot commented Apr 12, 2023

Codecov Report

Merging #2228 (67aba69) into master (2aee8fb) will not change coverage.
The diff coverage is n/a.

❗ Current head 67aba69 differs from pull request most recent head da5e0c9. Consider uploading reports for the commit da5e0c9 to get more accurate results

@@           Coverage Diff           @@
##           master    #2228   +/-   ##
=======================================
  Coverage   71.41%   71.41%           
=======================================
  Files          73       73           
  Lines        3348     3348           
  Branches       57       57           
=======================================
  Hits         2391     2391           
  Misses        954      954           
  Partials        3        3           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@msaroufim msaroufim merged commit a460afb into master Apr 12, 2023
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.

3 participants