Cache project config on demand #1000
Conversation
server/src/server.ts
Outdated
| if (changedPath.includes("build.ninja")) { | ||
| let projectRoot = utils.findProjectRootOfFile(changedPath); | ||
| if (projectRoot != null) { | ||
| syncProjectConfigCache(projectRoot); |
There was a problem hiding this comment.
maybe make this conditional on the setting, so potential issues of writing this file would be contained to having the setting on.
There was a problem hiding this comment.
Although the watch wouldn't be set up unless the setting is on.
analysis/src/Packages.ml
Outdated
| | None -> [] | ||
| | Some namespace -> | ||
| let cmt = Filename.concat libBs namespace ^ ".cmt" in | ||
| Hashtbl.add pathsForModule namespace (Namespace {cmt}); |
There was a problem hiding this comment.
When the cache file is read, pathsForModule will already have been populated when this line was executed when creating the cache, and here it is added again.
No consequence I can think of, but good practice to make sure things are not mutated after caching them.
There was a problem hiding this comment.
Good catch. Let me think about how to make that better.
There was a problem hiding this comment.
Maybe it's fine to leave in for now. I can't think of a consequence either. Maybe doing a Hashtbl.replace instead of add makes sense though.
analysis/bin/main.ml
Outdated
| | Some package -> Cache.cacheProject package | ||
| | None -> print_endline "\"ERR\"") | ||
| | [_; "cache-delete"; rootPath] -> ( | ||
| Cfg.useProjectConfigCache := false; |
There was a problem hiding this comment.
This does not seem to be used?
There was a problem hiding this comment.
There was a problem hiding this comment.
Sorry I meant to say the line Cfg.useProjectConfigCache := false does not seem to be necessary in the delete command, as none of the code below is affected by it.
This reverts commit bc71f76.
* cache project config on demand * e2e for the new cache mode * only look up project files etc when needed * comment * Revert "only look up project files etc when needed" This reverts commit bc71f76. * remove now irrelevant comment * changelog * conditionally * rename * replace instead of add
Experimental support for caching the project config to speed up analysis.
It should be good enough to merge and get started testing, and we can then polish before making it the default.
You enable it via the
rescript.settings.cache.projectConfig.enabledextension config option. It'll then watchbuild.ninjaand build a cache of (some of) the project config, that the analysis can then read as needed and avoid doing a bunch of work.In larger repos this has shown latency improvements of +8-9x. So a massive improvement.
Running it as experimental for a while to get some feedback, and then hopefully something that'll be easy and risk free to enable for everyone.