Skip to content
This repository has been archived by the owner on Apr 14, 2022. It is now read-only.

Keeping a ast.NodeVisitor compatible API may have performance limitations #14

Open
cabiad opened this issue Nov 15, 2018 · 1 comment
Open
Labels
enhancement New feature or request

Comments

@cabiad
Copy link
Contributor

cabiad commented Nov 15, 2018

See #1 (comment)

Another problem with the NodeVisitor API is that it couples registering a visitor subclass (batch of related node visitors) to walking (and visiting) the AST.

It's not clear to me whether re-walking each AST for each subclass would scale reasonably if we had dozens or hundreds of registered subclasses. In other words, would we linearly increase total execution time for a given set of ASTs or would some level of caching dominate, making the subsequent AST walks negligible.

If we see performance problems, a clear optimization path to consider is to walk once and call each visitor (method) registered for that node type

@cabiad cabiad added the enhancement New feature or request label Nov 15, 2018
@cabiad cabiad mentioned this issue Nov 15, 2018
4 tasks
@cabiad
Copy link
Contributor Author

cabiad commented Nov 20, 2018

Flame graph suggests that astroid.parse() is the primary slowdown. Walking the tree multiple times is not the slowest part of the process.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant