Skip to content

Some tweaks on how sketch is rebuild during editing #118

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

Merged
merged 6 commits into from
Jul 11, 2022
Merged

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented Jun 20, 2022

Please check if the PR fulfills these requirements

What kind of change does this PR introduce?
With this patch, we should gain some speed improvements in sketch rebuilds by caching libraries detection results from previous builds.

What is the current behavior?
When the sketch is rebuilt the libraries detection phase could take a very long time, especially if the sketch uses a lot of libraries or even a single library that contains a lot of files.

What is the new behavior?
The sketch is now rebuilt skipping the libraries detection and using the libraries detected on the latest "full" build.
A single "full" build is done when the language server is started, all subsequent builds will be without libraries detection. This means that #include <...> added after the language server startup may not be indexed/recognized until the next full build.

To trigger a new "full" build there are now two ways:

  1. restart the language server
  2. send a arduino/buildCompleted LSP notification from the IDE

The arduino/buildCompleted notification is an Arduino custom notification (it's not part of the LSP specification), it should be sent by the IDE to the language server after a full compile is triggered by the user of the IDE. Inside the notification, there is a buildPath field that should point to the path of the build directory so the language server can pick the result of libraries detection from there.

Other information
This PR uses a patched arduino-cli that support the SkipLibrariesDiscovery flag: arduino/arduino-cli#1777

@per1234 per1234 added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Jun 22, 2022
@kittaakos
Copy link
Contributor

We should look into Language Server Index Format Specification (LSIF):

The purpose of the Language Server Index Format (LSIF) is it to define a standard format for language servers or other programming tools to dump their knowledge about a workspace. This dump can later be used to answer language server LSP requests for the same workspace without running the language server itself.

@kittaakos
Copy link
Contributor

It looks promising.

In inols.log:

{"jsonrpc":"2.0","method":"arduino/buildCompleted","params":{"build_path":"/private/var/folders/z1/xkw1yh5n7rz4n8djprp1mdn80000gn/T/arduino-sketch-2A25CBEDDFEB3508AE8DDDA45D126AAA"}}Content-Length: 58

In inols-err.log: (I do not know if it's promising but it's there)

15:15:04.433519 �[92mIDE --> LS NOTIF arduino/buildCompleted�[0m
15:15:04.433562 �[92m                 arduino/buildCompleted: �[93mwrite-locked�[0m�[0m
15:15:04.475205 �[92m                 arduino/buildCompleted: �[93mwrite-unlocked�[0m�[0m

@cmaglie cmaglie force-pushed the speed_improvements branch 4 times, most recently from 8d531b9 to 98c8115 Compare July 5, 2022 13:32
@cmaglie
Copy link
Member Author

cmaglie commented Jul 5, 2022

I've included most of your changes here. We should probably document better the ino/didCompleteBuild custom command.

@cmaglie cmaglie marked this pull request as ready for review July 5, 2022 14:16
@cmaglie cmaglie force-pushed the speed_improvements branch from f4e9cb1 to 54a4e45 Compare July 11, 2022 14:03
cmaglie added 6 commits July 11, 2022 16:05
A full build is made only at language server startup.
Other full builds are performed by the IDE and build_path should be
passed to the LS with a custom command "ino/didCompleteBuild".

Co-authored-by: a.kitta@arduino.cc
Copy link
Contributor

@kittaakos kittaakos left a comment

Choose a reason for hiding this comment

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

Thank you so much for the new feature.

@cmaglie cmaglie merged commit 23a0f04 into main Jul 11, 2022
@cmaglie cmaglie deleted the speed_improvements branch July 11, 2022 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants