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

AST for original file is parsed repeatedly during bisection #212

Closed
akaihola opened this issue Sep 30, 2021 · 1 comment · Fixed by #214
Closed

AST for original file is parsed repeatedly during bisection #212

akaihola opened this issue Sep 30, 2021 · 1 comment · Fixed by #214
Labels
performance Speed or memory usage improvement
Milestone

Comments

@akaihola
Copy link
Owner

akaihola commented Sep 30, 2021

If AST verification fails, Darker starts to bisect in order to find the smallest possible number of extra diff context lines which preserves the AST. As noted by @rogalski #205, on each iteration of the loop in which AST verification fails,

Going back to black.assert_equivalent:
https://github.com/psf/black/blob/22747a6937d53e38397e96c4ed5ed0571db31f71/src/black/__init__.py#L1201

we'll call parse_ast for src repetitively, even though for single file reformatted in darker passed value will never change

Parsing the non-changing source file repeatedly is of course redundant. We could get rid of redundant calls and improve performance e.g. by

  • making the stringify_ast(parse_ast(src_ast)) call just once directly from Darker,
  • calling stringify_ast(parse_ast(dst_ast)) in Darker's bisection loop, and
  • comparing the results in the loop in order to detect whether the AST is intact.

While doing that, we could also get rid of unnecessary debug dumps which also harm performance (see #211 [closed by merged #214]).

The downside of re-implementing AST verification in Darker is that we wouldn't automatically benefit from any possible future refinements in Black to that code.

@akaihola akaihola added the performance Speed or memory usage improvement label Sep 30, 2021
@akaihola akaihola added this to the 1.3.2 milestone Oct 5, 2021
@akaihola akaihola modified the milestones: 1.3.2, 1.4.0 Oct 28, 2021
@akaihola akaihola linked a pull request Oct 31, 2021 that will close this issue
@akaihola
Copy link
Owner Author

This was resolved in #214.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Speed or memory usage improvement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant