Skip to content

Improve lintrunner job runtime #14240

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Sep 12, 2025
Merged

Improve lintrunner job runtime #14240

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6cb1b8e
Update
swolchok Sep 11, 2025
cd18132
Update
swolchok Sep 12, 2025
aee50cc
don't waste time doing another lintrunner init
swolchok Sep 12, 2025
e1a3133
port get-changed-files from pytorch/pytorch
swolchok Sep 12, 2025
d93113d
syntax is hard
swolchok Sep 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 47 additions & 3 deletions .github/workflows/lint.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ concurrency:
cancel-in-progress: true

jobs:
lintrunner:
lintrunner-mypy:
uses: pytorch/test-infra/.github/workflows/linux_job_v2.yml@main
permissions:
id-token: write
Expand Down Expand Up @@ -50,9 +50,53 @@ jobs:

RC=0
# Run lintrunner on all files
if ! lintrunner --force-color --all-files --tee-json=lint.json 2> /dev/null; then
if ! lintrunner --force-color --all-files --take MYPY --tee-json=lint.json 2> /dev/null; then
echo ""
echo -e "\e[1m\e[36mYou can reproduce these results locally by using \`lintrunner --take MYPY\`. (If you don't get the same results, run \'lintrunner init\' to update your local linter)\e[0m"
echo -e "\e[1m\e[36mSee https://github.com/pytorch/pytorch/wiki/lintrunner for setup instructions.\e[0m"
RC=1
fi

# Use jq to massage the JSON lint output into GitHub Actions workflow commands.
jq --raw-output \
'"::\(if .severity == "advice" or .severity == "disabled" then "warning" else .severity end) file=\(.path),line=\(.line),col=\(.char),title=\(.code) \(.name)::" + (.description | gsub("\\n"; "%0A"))' \
lint.json || true

exit $RC

lintrunner:
uses: pytorch/test-infra/.github/workflows/linux_job_v2.yml@main
permissions:
id-token: write
contents: read
with:
runner: linux.2xlarge
docker-image: ci-image:executorch-ubuntu-22.04-linter
submodules: 'recursive'
fetch-depth: 0
ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }}
timeout: 90
script: |
# The generic Linux job chooses to use base env, not the one setup by the image
CONDA_ENV=$(conda env list --json | jq -r ".envs | .[-1]")
conda activate "${CONDA_ENV}"

CACHE_DIRECTORY="/tmp/.lintbin"
# Try to recover the cached binaries
if [[ -d "${CACHE_DIRECTORY}" ]]; then
# It's ok to fail this as lintrunner init would download these binaries
# again if they do not exist
cp -r "${CACHE_DIRECTORY}" . || true
fi

# This has already been cached in the docker image
lintrunner init

RC=0
# Run lintrunner on all files
if ! lintrunner --force-color --all-files --skip MYPY --tee-json=lint.json 2> /dev/null; then
Copy link
Contributor

@digantdesai digantdesai Sep 11, 2025

Choose a reason for hiding this comment

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

I was thinking what if instead of --all-files can we run this on "changed" files in this PR as a required, not sure if that helps with MYPY performance or not..

Copy link
Contributor Author

@swolchok swolchok Sep 11, 2025

Choose a reason for hiding this comment

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

the problem isn't MYPY performance, it's the time to build and install executorch. check out some recent lintrunner runs on HUD: https://hud.pytorch.org/hud/pytorch/executorch/main/1?per_page=50&name_filter=lintrunner

Copy link
Contributor Author

@swolchok swolchok Sep 11, 2025

Choose a reason for hiding this comment

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

instead of --all-files can we run this on "changed" files in this PR

good idea, PyTorch's lint.yml does something like this, but it's out of scope for this PR

Copy link
Contributor Author

@swolchok swolchok Sep 12, 2025

Choose a reason for hiding this comment

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

on second thought, this is a good idea and I'm open to just increasing scope of this PR so that we can be confident that we actually get a speed improvement.

echo ""
echo -e "\e[1m\e[36mYou can reproduce these results locally by using \`lintrunner\`. (If you don't get the same results, run \'lintrunner init\' to update your local linter)\e[0m"
echo -e "\e[1m\e[36mYou can reproduce these results locally by using \`lintrunner --skip MYPY\`. (If you don't get the same results, run \'lintrunner init\' to update your local linter)\e[0m"
echo -e "\e[1m\e[36mSee https://github.com/pytorch/pytorch/wiki/lintrunner for setup instructions.\e[0m"
RC=1
fi
Expand Down
Loading