Skip to content

Presubmit Critical Path Gating (GitHub Workflow)#341

Merged
haochengd-google merged 14 commits into
mainfrom
presubmit
Jul 17, 2026
Merged

Presubmit Critical Path Gating (GitHub Workflow)#341
haochengd-google merged 14 commits into
mainfrom
presubmit

Conversation

@haochengd-google

@haochengd-google haochengd-google commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

This PR is a follow-up to comment in PR 290. The CL has been approved and submitted (reference numbers, 947929859 and 948702917), and this is the PR1 submission. Two workflows have been tested in this test branch test 1 and test 2.

[!Note] Please note that only push will trigger Gating in the current setting. Please let me know if we also would like to trigger Gating when there is a pull_request.

Files

CI workflows

  • .github/workflows/run_benchmarks.yml — presubmit gate workflow (runs both benchmarks via BAP on PRs)
  • .github/workflows/update_d2h_h2d_baseline.yml — manually-triggerable record workflow that rewrites the baselines file

Build infrastructure (needed to build the workload in CI)

  • .bazelrc — adds the oss and ci build configs (toolchain/flags, RBE remote cache, and the torch_tpu module override)

@haochengd-google
haochengd-google requested review from linamy85 and removed request for linamy85 July 16, 2026 04:42

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There was a recent fix to recover performance on KvCacheManager, which might improve the baseline. Do we need to update the baseline?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for reminding me! I have observed this, I will submit another CL to update the baseline.

name: Run Benchmarks

on:
workflow_dispatch:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What workflow is this?
And is this triggered on presubmit? (I'm thinking that the benchmark should be launched safely and avoid abuse by external users - e.g. when approvals are granted, or if it's generated by copybara)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for flagging this — good to double-check.

What it is: this is the presubmit gating workflow from the PR-290 split (PR 1). It runs the H2D/D2H and H2H transfer benchmarks through BAP on a self-hosted TPU runner to catch perf regressions.

Triggers: it only runs on push to main (plus a nightly schedule and manual workflow_dispatch) — there's no pull_request trigger. Since main is copybara-managed, push only fires on copybara exports that were already reviewed internally, so external PRs can't launch the self-hosted TPU runner.

If you'd like a pull_request trigger too, happy to add one with a guard (e.g. author_association / approval-required) so untrusted PRs can't run it. Let me know what you'd prefer!

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Got it, I think we might soon need the capability to accept external PRs. Having such guard (approval-required) should be helpful, but let's follow-up in separate CL, and first enable VM-based presubmit guard for internal developers.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Also just to double check, are you saying that the push means when critique cl is created, the external presubmit will be triggered? Or is it post-submit only?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It is supposed to trigger a presubmit. I will create a CL to test once it is merged, thanks!

@haochengd-google
haochengd-google merged commit 5351729 into main Jul 17, 2026
8 checks passed
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.

2 participants