Skip to content

perf(formatter): Skip bodies without comments#4978

Merged
MichaReiser merged 1 commit intomainfrom
perf-skip-body
Jun 9, 2023
Merged

perf(formatter): Skip bodies without comments#4978
MichaReiser merged 1 commit intomainfrom
perf-skip-body

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Jun 9, 2023 •
edited
Loading

Summary

This PR improves the performance of the comments extraction by skipping over bodies that contain no comments.

The visitor already avoided visiting the children of a node if there are no comments in its range but we aren't doing this yet for bodies because bodies are not nodes.
This PR adds the logic for skipping over bodies if there's no comment in the body range.

if test: # a comment 
	...

The old implementation did visit the if statement, as well as the body and orelse because it contains a comment in its range. But this is unnecessary
because only the case header has a comment.

Test Plan

cargo test

This was referenced Jun 9, 2023
Merged
Merged
Merged
Merged
Merged
Merged
@MichaReiser
Copy link
Member Author

MichaReiser commented Jun 9, 2023 •
edited
Loading

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@MichaReiser MichaReiser mentioned this pull request Jun 9, 2023
Merged
@MichaReiser MichaReiser changed the title perf(formatter): Avoid skipping bodies without comments perf(formatter): Skip bodies without comments Jun 9, 2023
@MichaReiser MichaReiser added internal An internal refactor or improvement performance Potential performance improvement formatter Related to the formatter labels Jun 9, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jun 9, 2023 •
edited
Loading

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.02      6.5±0.02ms     6.3 MB/sec    1.00      6.4±0.01ms     6.4 MB/sec
formatter/numpy/ctypeslib.py               1.01   1357.2±7.20µs    12.3 MB/sec    1.00   1339.5±2.51µs    12.4 MB/sec
formatter/numpy/globals.py                 1.01    131.5±0.23µs    22.4 MB/sec    1.00    129.9±0.17µs    22.7 MB/sec
formatter/pydantic/types.py                1.02      2.7±0.02ms     9.6 MB/sec    1.00      2.6±0.02ms     9.7 MB/sec
linter/all-rules/large/dataset.py          1.00     14.0±0.02ms     2.9 MB/sec    1.00     14.0±0.04ms     2.9 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.4±0.00ms     4.9 MB/sec    1.00      3.4±0.01ms     4.9 MB/sec
linter/all-rules/numpy/globals.py          1.00    423.2±0.61µs     7.0 MB/sec    1.00    423.3±0.60µs     7.0 MB/sec
linter/all-rules/pydantic/types.py         1.00      5.9±0.01ms     4.3 MB/sec    1.00      5.9±0.01ms     4.3 MB/sec
linter/default-rules/large/dataset.py      1.00      6.7±0.01ms     6.0 MB/sec    1.01      6.8±0.01ms     6.0 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00   1471.2±3.38µs    11.3 MB/sec    1.01   1484.2±2.40µs    11.2 MB/sec
linter/default-rules/numpy/globals.py      1.00    166.1±0.29µs    17.8 MB/sec    1.00    166.7±0.23µs    17.7 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.1±0.01ms     8.3 MB/sec    1.00      3.1±0.01ms     8.3 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      7.6±0.06ms     5.3 MB/sec    1.15      8.7±0.07ms     4.7 MB/sec
formatter/numpy/ctypeslib.py               1.00  1556.5±15.14µs    10.7 MB/sec    1.12  1739.9±23.82µs     9.6 MB/sec
formatter/numpy/globals.py                 1.00    149.3±3.21µs    19.8 MB/sec    1.11    165.5±4.76µs    17.8 MB/sec
formatter/pydantic/types.py                1.00      3.1±0.04ms     8.2 MB/sec    1.13      3.5±0.04ms     7.2 MB/sec
linter/all-rules/large/dataset.py          1.00     16.4±0.10ms     2.5 MB/sec    1.01     16.6±0.12ms     2.5 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      4.2±0.05ms     4.0 MB/sec    1.01      4.2±0.04ms     3.9 MB/sec
linter/all-rules/numpy/globals.py          1.00    497.1±5.72µs     5.9 MB/sec    1.00    499.3±6.78µs     5.9 MB/sec
linter/all-rules/pydantic/types.py         1.00      7.0±0.05ms     3.6 MB/sec    1.01      7.1±0.06ms     3.6 MB/sec
linter/default-rules/large/dataset.py      1.00      8.2±0.04ms     5.0 MB/sec    1.00      8.2±0.06ms     4.9 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1755.9±29.79µs     9.5 MB/sec    1.00  1747.9±23.44µs     9.5 MB/sec
linter/default-rules/numpy/globals.py      1.00    198.1±2.83µs    14.9 MB/sec    1.01    200.1±6.00µs    14.7 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.7±0.03ms     6.8 MB/sec    1.00      3.7±0.03ms     6.9 MB/sec

@MichaReiser
Copy link
Member Author

MichaReiser commented Jun 9, 2023

The benchmarks compare against main which does not yet have the if statement formatting. But we can see that the increase goes down from 25% slowdown to an at most 18% slowdown.

@MichaReiser MichaReiser requested a review from konstin June 9, 2023 07:17
@konstin
Copy link
Member

konstin commented Jun 9, 2023

With #4964 (comment), i don't really see much performance difference

@MichaReiser MichaReiser force-pushed the fix-bin-oip-with-leading-comments branch 2 times, most recently from 7458b5b to e341c1c Compare June 9, 2023 08:56
Base automatically changed from fix-bin-oip-with-leading-comments to main June 9, 2023 09:02
@MichaReiser
perf(formatter): Avoid skipping bodies without comments
2b36a62 
Improve the performance of the comments extraction by skipping bodies
that contain no comments.
@MichaReiser
Copy link
Member Author

MichaReiser commented Jun 9, 2023

With #4964 (comment), i don't really see much performance difference

The linux result are in the range of what I expected and was able to reproduce locally. Not sure what happened with the windows run.

@MichaReiser MichaReiser merged commit 111e1f9 into main Jun 9, 2023
@MichaReiser MichaReiser deleted the perf-skip-body branch June 9, 2023 09:33
konstin pushed a commit that referenced this pull request Jun 13, 2023
@MichaReiser @konstin
perf(formatter): Skip bodies without comments (#4978)
3efcbc6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@konstin konstin konstin approved these changes

Assignees

No one assigned

Labels

formatter Related to the formatter internal An internal refactor or improvement performance Potential performance improvement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@MichaReiser @konstin