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

Add GPU profiler #1997

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Add GPU profiler #1997

wants to merge 3 commits into from

Conversation

jainapurva
Copy link
Contributor

@jainapurva jainapurva commented Apr 1, 2025

  • Introduces functions to upload trace files, generate Perfetto UI URLs, and profile model and memory usage.
  • Adds comprehensive tests to validate profiler and memory profiling functionalities.
  • Updates benchmark configuration and runner logic to integrate these new profiling features.

@jainapurva
Copy link
Contributor Author

jainapurva commented Apr 1, 2025

Copy link

pytorch-bot bot commented Apr 1, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/ao/1997

Note: Links to docs will display an error until the docs builds have been completed.

❌ 2 New Failures

As of commit 328b7bf with merge base 70fc520 (image):

NEW FAILURES - The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 1, 2025
jainapurva added a commit that referenced this pull request Apr 1, 2025
)

ghstack-source-id: a5f8301acb77a180a395aa8dd4c1aa9c2ccd7522
ghstack-comment-id: 2770609971
Pull Request resolved: #1997
@jainapurva jainapurva changed the title Add profiler and Perfetto UI link with comprehensive tests (#1984, #1992) Add profiler and Perfetto UI link with comprehensive tests Apr 1, 2025
@jainapurva jainapurva requested a review from Copilot April 1, 2025 20:33
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request adds profiling capabilities and integrates a Perfetto UI link to the benchmark framework while also enhancing memory profiling and test coverage.

  • Introduces functions to upload trace files, generate Perfetto UI URLs, and profile model and memory usage.
  • Adds comprehensive tests to validate profiler and memory profiling functionalities.
  • Updates benchmark configuration and runner logic to integrate these new profiling features.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
benchmarks/microbenchmarks/utils.py New functions added for trace file upload, Perfetto URL generation, and model/memory profiling.
benchmarks/microbenchmarks/test/test_benchmark_profiler.py New tests added to verify profiler and memory profiling functionality.
benchmarks/microbenchmarks/test/benchmark_config.yml Updated configuration to enable profiling for specific models.
benchmarks/microbenchmarks/benchmark_runner.py Enhanced error handling and conditional inclusion of benchmark results.
benchmarks/microbenchmarks/benchmark_inference.py Integrated profiling steps into the inference benchmark run.
Comments suppressed due to low confidence (2)

benchmarks/microbenchmarks/utils.py:95

            "https://interncache-all.fbcdn.net/manifold/perfetto-artifacts/tree/ui/index.html#!/?url=https://interncache-all.fbcdn.net/manifold/" + manifold_path

benchmarks/microbenchmarks/utils.py:81

  • [nitpick] Consider adding a test case to simulate a failure in upload_trace_file (e.g., by mocking subprocess.run to return a non-zero code) to ensure that error handling in generate_model_profile works as expected.
        print("[ERROR] Upload failed, maybe the trace file exists.")

@jainapurva jainapurva changed the title Add profiler and Perfetto UI link with comprehensive tests Add GPU profiler Apr 1, 2025
)

ghstack-source-id: a5f8301acb77a180a395aa8dd4c1aa9c2ccd7522
ghstack-comment-id: 2770609971
Pull Request resolved: #1997
@jainapurva jainapurva force-pushed the gh/jainapurva/25/head branch from ace610f to 967ea76 Compare April 4, 2025 06:00
@jainapurva jainapurva mentioned this pull request Apr 4, 2025
@jainapurva jainapurva mentioned this pull request Apr 4, 2025
@jainapurva jainapurva force-pushed the gh/jainapurva/25/head branch from 4acb7e8 to dd9f50d Compare April 4, 2025 16:55
[ghstack-poisoned]
This was referenced Apr 4, 2025
@jainapurva jainapurva marked this pull request as draft April 4, 2025 19:56
@jainapurva jainapurva requested a review from Copilot April 7, 2025 17:51
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

DEFAULT_TTL_SEC = 28 * 24 * 60 * 60
file_name = os.path.basename(local_path)
manifold_path = os.path.join(
MANIFOLD_FOLDER, f"{os.getlogin()}_{str(uuid.uuid4())}_{file_name}"
Copy link
Preview

Copilot AI Apr 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using getpass.getuser() instead of os.getlogin() to handle non-interactive environments better, as os.getlogin() may fail in such contexts.

Suggested change
MANIFOLD_FOLDER, f"{os.getlogin()}_{str(uuid.uuid4())}_{file_name}"
MANIFOLD_FOLDER, f"{getpass.getuser()}_{str(uuid.uuid4())}_{file_name}"

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants