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

Changes to table constructor formatting severely degraded performance #205

Closed
JohnnyMorganz opened this issue Jun 20, 2021 · 0 comments · Fixed by #208
Closed

Changes to table constructor formatting severely degraded performance #205

JohnnyMorganz opened this issue Jun 20, 2021 · 0 comments · Fixed by #208
Labels
bug Something isn't working

Comments

@JohnnyMorganz
Copy link
Owner

In fbe55eb (#182), we changed the formatting tactic for table constructors.
Originally, to see if a table constructor is over the column width, we would check the difference between the position of the start and end brace, and add it to the current width taken. This was quite naiive, because if you had a badly formatted table constructor with lots of spaces, it would go over the width.

To change this, we first attempt to format the whole table on a single line. If it then goes over width, then we stop, scrap the formatting we did, and then format on multiple lines. For code with lots of (nested) tables, this degraded performance significantly, as we double the amount of (quite expensive) work done per table, which adds up exponentially as you increase the depth of a table.

Reverting to a snapshot before this commit heavily improves performance, therefore a new heuristic needs to be defined for this.
We could either revert to the tactic we had originally (which still isnt great), or we could find something else.
Maybe, rather than attempting to format the whole table singleline then check if its over width, we format a small chunk at a time, examine how far we are, and bail early. This short circuiting should reduce the amount of operations done significantly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant