Skip to content

Conversation

@huydhn
Copy link
Contributor

@huydhn huydhn commented Oct 2, 2024

This PR adds a job to upload Android benchmark results to the benchmark database. It transforms the benchmark_results.json file slightly to fit into the current schema. We are going to have a better schema soon https://fburl.com/gdoc/ossgtvte, but landing this first would unblock the work on building the dashboard before the launch. Updating the schema can be done later.

  • The job processes what it finds, so if one model fails, the rest will still be uploaded.
  • I will follow up with another PR for iOS later. No need to wait for the TPS metric there, we'll upload what available first.

There are still some TODO pending:

But the structure of the CI job is ready to review.

@pytorch-bot
Copy link

pytorch-bot bot commented Oct 2, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/5808

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

✅ No Failures

As of commit c942ab4 with merge base 2f9f94a (image):
💚 Looks good so far! There are no failures yet. 💚

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 Oct 2, 2024
huydhn added a commit to pytorch/test-infra that referenced this pull request Oct 3, 2024
This updates the mobile job to return the list of artifacts from AWS to
the caller to use as they see fit. One of the use case is for ExecuTorch
to extract the benchmark results JSON from `CUSTOMER_ARTIFACT` and
`TESTSPEC_OUTPUT` (2 types of artifacts from AWS).

My plan here is to:

* [x] Get the list of artifacts (this PR)
* [ ] On ExecuTorch side, extract the benchmark JSON from the artifacts
pytorch/executorch#5808
* [ ] Create a new GitHub action on test-infra to upload the JSON (This
could be done on ExecuTorch side too, but I plan to make this generic
and reusable by other projects, so it must be on test-infra)

Minor fixes:
* Also update the console output to print out the context of
`TESTSPEC_OUTPUT`, which is basically the main output from the device.
* Print additional information about the job name (the device name from
AWS in disguise) and app type (Android, iOS)
* Some lint fix here and there.
@huydhn huydhn marked this pull request as ready for review October 3, 2024 23:28
@facebook-github-bot
Copy link
Contributor

@huydhn has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Copy link
Contributor

@guangy10 guangy10 left a comment

Choose a reason for hiding this comment

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

LGTM

@huydhn huydhn had a problem deploying to upload-benchmark-results October 5, 2024 00:17 — with GitHub Actions Failure
@huydhn huydhn had a problem deploying to upload-benchmark-results October 5, 2024 00:50 — with GitHub Actions Failure
@huydhn huydhn had a problem deploying to upload-benchmark-results October 5, 2024 01:28 — with GitHub Actions Failure
@huydhn huydhn temporarily deployed to upload-benchmark-results October 5, 2024 03:37 — with GitHub Actions Inactive
@huydhn
Copy link
Contributor Author

huydhn commented Oct 5, 2024

The testing looks good https://github.com/pytorch/executorch/actions/runs/11189433899/job/31111425608, I can see the data on the database.

huydhn added a commit to pytorch/test-infra that referenced this pull request Oct 5, 2024
This adds a GHA to upload benchmark results. I'm using the existing
`torchci-oss-ci-benchmark` database for now until the new generic
database from https://fburl.com/gdoc/ossgtvte is ready. So, it's backed
by dynamoDB instead of S3 for now.

I also include a quick test with some dummy Android benchmark records.

* [x] Get the list of artifacts
#5727
* [x] On ExecuTorch side, extract the benchmark JSON from the artifacts
pytorch/executorch#5808
* [x] Create a new GitHub action on test-infra to upload the JSON
#5742
* [x] Create a new GHA role to upload the results
meta-pytorch/pytorch-gha-infra#483
@facebook-github-bot
Copy link
Contributor

@huydhn has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@huydhn huydhn temporarily deployed to upload-benchmark-results October 5, 2024 04:27 — with GitHub Actions Inactive
Summary:
This PR adds a job to upload Android benchmark results to the benchmark database.  It transforms the `benchmark_results.json` file slightly to fit into the current schema.  We are going to have a better schema soon https://fburl.com/gdoc/ossgtvte, but landing this first would unblock the work on building the dashboard before the launch.  Updating the schema can be done later.

* The job processes what it finds, so if one model fails, the rest will still be uploaded.
* I will follow up with another PR for iOS later. No need to wait for the TPS metric there, we'll upload what available first.

There are still some TODO pending:

* pytorch/test-infra#5742
* meta-pytorch/pytorch-gha-infra#483

But the structure of the CI job is ready to review.

Pull Request resolved: #5808

Reviewed By: guangy10, kirklandsign

Differential Revision: D63869876

Pulled By: huydhn
@huydhn huydhn force-pushed the upload-android-perf-results branch from 542db36 to c942ab4 Compare October 5, 2024 05:27
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D63869876

@facebook-github-bot
Copy link
Contributor

@huydhn merged this pull request in 4651d65.

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. fb-exported Merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants