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

Black runs slow debug code on AST verification failure #211

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

Black runs slow debug code on AST verification failure #211

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,

verify_ast_unchanged [...] invokes quite slow dump debug logic, which should be skipped in our bisect, as we expect some non-equivalent ASTs in a process

The slow code is in black.output.dump_to_file() called from black.assert_equivalent() in two cases:

  • the reformatted file can't be parsed
  • the parsed AST of the reformatted file differs from the one of the original file

As we expect at least the latter to happen during bisection, we could try to avoid invoking the slow debug code.

One way to do this would be to implement our own version of assert_equivalent(). See also #212 for another optimization which this would enable.

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.4.0 milestone Sep 30, 2021
@akaihola
Copy link
Owner Author

@rogalski, continuing the discussion from #205:

About slow debug code - I am mostly talking about stringify_ast in black: https://github.com/psf/black/blob/22747a6937d53e38397e96c4ed5ed0571db31f71/src/black/__init__.py#L1220

But isn't stringify_ast() an essential operation which produces the objects to compare from source and destination ASTs? To my eye it isn't debug code (like dump_to_file() is).

As for dump_to_file(), I fully agree, getting rid of that call would probably improve performance.

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