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 lexing and parsing from the linter benchmark #9264

Merged
merged 1 commit into from
Dec 23, 2023

Conversation

charliermarsh
Copy link
Member

Summary

This PR adds some helper structs to the linter paths to enable passing in the pre-computed tokens and parsed source code during benchmarking, to remove lexing and parsing from the overall linter benchmark measurement. We already remove parsing for the formatter, and we have separate benchmarks for the lexer and the parser, so this should make it much easier to measure linter performance changes.

@charliermarsh charliermarsh added the internal An internal refactor or improvement label Dec 23, 2023
Copy link

codspeed-hq bot commented Dec 23, 2023

CodSpeed Performance Report

Merging #9264 will improve performances by ×4.9

Comparing charlie/benchmark (30c83ae) with main (20def33)

Summary

⚡ 15 improvements
✅ 15 untouched benchmarks

Benchmarks breakdown

Benchmark main charlie/benchmark Change
linter/default-rules[unicode/pypinyin.py] 6 ms 1.5 ms ×4.1
linter/default-rules[large/dataset.py] 92.1 ms 19 ms ×4.9
linter/default-rules[numpy/globals.py] 1,799.9 µs 634.8 µs ×2.8
linter/all-rules[large/dataset.py] 178 ms 101.9 ms +74.62%
linter/default-rules[pydantic/types.py] 36.4 ms 8.4 ms ×4.3
linter/all-with-preview-rules[numpy/globals.py] 4.5 ms 3.4 ms +31.16%
linter/all-rules[pydantic/types.py] 77.1 ms 48.2 ms +60.21%
linter/all-with-preview-rules[pydantic/types.py] 85.9 ms 56.2 ms +52.92%
linter/all-with-preview-rules[unicode/pypinyin.py] 17.8 ms 13.3 ms +33.86%
linter/all-with-preview-rules[numpy/ctypeslib.py] 39.4 ms 26.3 ms +49.92%
linter/all-rules[numpy/ctypeslib.py] 36.1 ms 23.2 ms +55.54%
linter/all-rules[numpy/globals.py] 4.2 ms 3 ms +39.02%
linter/all-with-preview-rules[large/dataset.py] 197 ms 122 ms +61.46%
linter/all-rules[unicode/pypinyin.py] 16.7 ms 12.2 ms +37.28%
linter/default-rules[numpy/ctypeslib.py] 17.1 ms 4 ms ×4.3

@charliermarsh charliermarsh merged commit 9d64441 into main Dec 23, 2023
17 checks passed
@charliermarsh charliermarsh deleted the charlie/benchmark branch December 23, 2023 21:43
@charliermarsh
Copy link
Member Author

Just gonna merge since folks are out, I'll handle any feedback in follow-up PRs if it comes in.

Copy link
Contributor

ruff-ecosystem results

Linter (stable)

ℹ️ ecosystem check encountered linter errors. (no lint changes; 1 project error)

pypa/setuptools (error)

ruff failed
  Cause: 'quote-style = preserve' is a preview only feature. Run with '--preview' to enable it.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+1 -1 violations, +0 -0 fixes in 1 projects; 40 projects unchanged)

python/mypy (+1 -1 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview

+ mypy/stubtest.py:1396:27: E999 SyntaxError: unexpected EOF while parsing
- mypy/stubtest.py:680:21: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks

Changes by rule (2 rules affected)

code total + violation - violation + fix - fix
E999 1 1 0 0 0
E721 1 0 1 0 0

Formatter (stable)

ℹ️ ecosystem check encountered format errors. (no format changes; 1 project error)

pypa/setuptools (error)

ruff failed
  Cause: 'quote-style = preserve' is a preview only feature. Run with '--preview' to enable it.

Formatter (preview)

✅ ecosystem check detected no format changes.

@charliermarsh
Copy link
Member Author

Not sure I understand the Mypy ecosystem change, hmm... (The setuptools error is already occurring on main and is unrelated.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal An internal refactor or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant