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

Performance oriented fixes. #841

Merged
merged 10 commits into from
Dec 11, 2024
Merged

Performance oriented fixes. #841

merged 10 commits into from
Dec 11, 2024

Conversation

scohen
Copy link
Collaborator

@scohen scohen commented Dec 1, 2024

This commit fixes a number of performance related issues, and reworks formatting so it's lock-free and able to be used with format-on-save in any editor. The fixes are:

  • We only call deps.compile if the vm changes directories during compilation. This dramatically speeds up compiling erlang deps.
  • Adds a timing helper to time blocks of code
  • Precalculates contexts around the cursor. Fetching the current context was called once for every completion item. This meant that for large modules, like Enum, hundreds of context calculations were happening.
  • Reworked formatting: Jose added a couple of options to Mix.Tasks.Format.formatter_for_file that allow us to not need to be in the project context in order for formatting to work. This enables lock-free formatting which makes the amount of time formatting takes to be proportional to the length of the file. Formatting on save no longer waits on compilation, and is safe for any project

@scohen
Copy link
Collaborator Author

scohen commented Dec 1, 2024

I've been working on this branch for a couple of weeks and haven't noticed any regressions or issues. The improvements are drastic, especially on my project.

label main performance
project compilation 27s 1.8s
completion of Enum. 1.8s < 1ms
formatting worst case 4.7s 110ms

Notes:

  1. My project has a number of erlang deps, and each one would take 750ms to compile
  2. The improvement in formatting was eliminating waiting on the build so the project lock would become available.

Copy link
Collaborator

@Moosieus Moosieus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code checks out, and I'm continuing to use this branch without issue.

Would it make sense to change Lexical.Debug.log_timed/3 to debug level instead of info? That's the only outstanding consideration I have.

Note: this requires more work.

  * The locations of a project's .formatter.exs files should be
  cached, along with their mtimes so we can calculate their AST once.
  * Additionally, we need to check to see if the .formatter.exs is
  opened, and if it is, use that AST.
We were calculating the context of the cursor once per suggestion,
this would take a couple dozen milliseconds, but for a module that had
a lot of functions, this would add up. Precalculating them saved a lot
of time.
Jose added a function that allows us to get a formatter without having
to lock on the build. This reduces the formatting time significantly,
and we no longer have that frustrating freeze on save.
@scohen scohen merged commit a9a806c into main Dec 11, 2024
@scohen scohen deleted the performance branch December 11, 2024 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants