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

internal/mod/modimports: loading of CUE syntax needs a cache #3177

Closed
rogpeppe opened this issue May 27, 2024 · 0 comments
Closed

internal/mod/modimports: loading of CUE syntax needs a cache #3177

rogpeppe opened this issue May 27, 2024 · 0 comments
Labels
modules Issues related to CUE modules and the experimental implementation NeedsFix

Comments

@rogpeppe
Copy link
Member

This up-coming change to modimports (https://cuelang.org/cl/1195280) to make it read packages spread across
multiple directories means that it will be doing redundant work reading and parsing CUE files
in shared parent directories. In the cue/load package, this issue was addressed by adding a cache (see https://cuelang.org/cl/1193678). The modimports code should be changed similarly to use a cache,
possibly shared with the one in cue/load.

One way of doing that might be to add an optional interface to the fs.FS instance passed into modimports
that provides access to parsed syntax trees.

@rogpeppe rogpeppe added NeedsFix modules Issues related to CUE modules and the experimental implementation labels May 27, 2024
@rogpeppe rogpeppe changed the title internal/mod/modimports: loading of CUE syntax could use a cache internal/mod/modimports: loading of CUE syntax needs a cache May 27, 2024
cueckoo pushed a commit that referenced this issue Jul 11, 2024
In order to to fix issue #3177, we need a cache that's shared between
the module code in `internal/mod/...` and the `cue/load` package, which
means that the syntax cache needs to be pushed down to a lower level in
`cue/load` because currently it's based around cue/load-specific
abstractions.

This CL is an incremental progression towards that goal: it adds a
`getCUESyntax` method to the internal filesystem abstraction in
`cue/load`, which will become a cache in a subsequent CL.

In order to do this, some refactoring was required, namely removal of
the `matchFile` function, because that function read data without
parsing the CUE, but the parsing and reading operations now need to be
implemented by the same operation (the new `getCUESyntax` method). In
any case, the `matchFile` function did not really fulfil its original
purpose or fit the name any more.

Also remove the return value from `fileProcessor.add` while we're
refactoring it, because nothing uses it.

For #3177.

Signed-off-by: Roger Peppe <[email protected]>
Change-Id: I1876f22b99a5c611e3d8650d3b7afb8f8e43f202
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1197525
Unity-Result: CUE porcuepine <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Reviewed-by: Paul Jolly <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
modules Issues related to CUE modules and the experimental implementation NeedsFix
Projects
Archived in project
Status: v0.10.0-alpha.1
Development

No branches or pull requests

1 participant