Conversation
| # the idle timeout does not stop it and force a full recompile on the | ||
| # next request. | ||
| if result notin ls.entryPoints: | ||
| ls.entryPoints.add result |
There was a problem hiding this comment.
A function named getProjectFile should probably not do this mutation.
| if projectFile in ls.startingProjects: | ||
| debug "Nimsuggest is already starting, not spawning another one", | ||
| projectFile = projectFile, uri = uri | ||
| return |
There was a problem hiding this comment.
this changes the behavior for callers of createOrRestartNimsuggest, as it can return before created/restarted. Is it safe to do so?
There is another approach here https://github.com/nitely/langserver/blob/7dcea2c096f163af3e0bf2da4048458b72298639/ls.nim#L1146 tht should keep the current behavior; and the behavior looks more consistent across calls; but does not do it for entryPoints.
There was a problem hiding this comment.
There was a problem hiding this comment.
Also, try adding some regression test.
nitely@b0b7989 contains a few tests on the line of this PR + some related fixes (specially one that "shields" (using the "join" chronos API) the starting nimsuggest that is shared across requests, so a request cancelation wont cancel the starting numsuggest).
There was a problem hiding this comment.
Thanks for the feedback for the links. I updated my PR. Basically instructed the LLM to copy your design from the linked commits, so that the issues you pointed out were addressed: entryPoints are not mutation in getProjectFile, createOrRestartNimsuggest preserves behavior. Also, some tests were added.
fc8b67d to
c891028
Compare
2 participants
This PR grows out of the Constantine issue.
Constantine is a multi-entry-point project: instead of having one root module that reaches the whole package, it has several independent API and implementation entry points scattered throughout the repository.
To provide Nimsuggest with the appropriate compilation context, users can configure
nim.projectMapping. This makes the initial Nimsuggest compilation more expensive. For Constantine, the mapped project root takes approximately 15 seconds to compile on my machine.The language server previously had two lifecycle problems with such roots:
A project was not registered in
projectFilesuntil Nimsuggest had finished its initial compilation. Requests arriving during that period could start additional Nimsuggest instances for the same root. Those instances compiled the same large project in parallel, and starting one could stop a previously started instance.The language server waited only briefly for an instance to become available, even though initial compilation could take much longer.
This resulted in multiple Nimsuggest processes being started and then immediately stopped, followed by unreliable language-server requests.
This PR tracks project roots that are already being started, prevents duplicate Nimsuggest processes for the same root, and waits for slow initial compilations. It also treats explicitly mapped project roots like Nimble entry points so that the idle cleanup does not stop them and force another full compilation later.