julia-vscode / SymbolServer.jl

Other
23 stars 31 forks source link

refactor of how symbols are pulled from packages #154

Closed ZacLN closed 4 years ago

ZacLN commented 4 years ago

Changes how symbols are cached - now caches the entire environment as existing in the symbol server process all in one go rather than trying to treat modules as separate objects (they're not).

This appears to be significantly faster than the previous approach.

Adds a test to ensure all the symbols we need are cached.

Todo: Add test to check that function methods are assigned to the correct FunctionStores within each module.

davidanthoff commented 4 years ago

Does that mean that we need to re-index an environment completely even if only one package is updated? How would this interact with the plan to precompute cache files for individual packages in the cloud?

ZacLN commented 4 years ago

Not really, my description above wasn't the best. As before when running server.jl on the worker process we only load those packages for which the on-disc cache doesn't exist/is out of date but we now recreate (and save to the disc) caches for all packages that get loaded during this process (i.e. the dependencies). This doesn't add any computational effort though.

I don't see this as having any impact on the remote storage approach.

This also highlighted that we don't successfully cache TableTraits in our tests (it returns an empty ModuleStore). I'm not sure if this was intentional?

EDIT: this fixes https://github.com/julia-vscode/julia-vscode/issues/1224

ZacLN commented 4 years ago

This will require a re-caching of packages to take effect - as a mechanism for ensuring cache's get updated are we simply renaming the persistent store folder?

davidanthoff commented 4 years ago

Yes, the idea is that we just change https://github.com/julia-vscode/julia-vscode/blob/aeb23892f431f08f3b2d355772dc29cc4e933ecc/scripts/languageserver/main.jl#L43 to use symbolstorev2. We should eventually add some code (probably to the extension) that deletes old folders at startup if they are present, but I think we could add that feature later.

davidanthoff commented 4 years ago

@ZacLN are you opening a PR against julia-vsocde that updates the symserver path?

ZacLN commented 4 years ago

Yep