Skip to content

_stream: Only query cache for the requested elements#1965

Merged
gtristan merged 1 commit intomasterfrom
abderrahim/minimal-cache-query
Dec 11, 2024
Merged

_stream: Only query cache for the requested elements#1965
gtristan merged 1 commit intomasterfrom
abderrahim/minimal-cache-query

Conversation

@abderrahim
Copy link
Contributor

@abderrahim abderrahim commented Oct 31, 2024

query_cache() used to query the cache for the elements passed and all their (build and runtime) dependencies. This is wasteful since most of the time, we don't need all this cache querying. And for the cases we need, it is the job of the query_cache() caller to include the dependencies they need.

As it turns out, this is only needed by bst show to be able to figure out the cache key in all circumsances.

This commit makes this behaviour optional, and only enables it from bst show.

@juergbi
Copy link
Contributor

juergbi commented Nov 8, 2024

I think this makes sense. However, do I see this right that it doesn't make a difference for typical builds as we always load all elements to support dynamic planning (unless --deps build is specified)? So this is mainly an optimization for non-build commands, or am I missing something?

@abderrahim
Copy link
Contributor Author

I think this makes sense. However, do I see this right that it doesn't make a difference for typical builds as we always load all elements to support dynamic planning (unless --deps build is specified)? So this is mainly an optimization for non-build commands, or am I missing something?

That's correct. The main optimisation here is for artifact checkout.

The commit message here hasn't been updated to cover my most recent findings. I'll try to update it later today.

Copy link
Contributor

@gtristan gtristan left a comment

Choose a reason for hiding this comment

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

LGTM

@abderrahim abderrahim force-pushed the abderrahim/minimal-cache-query branch from a960325 to 65b1807 Compare December 11, 2024 14:10
query_cache() used to query the cache for the elements passed and all
their (build and runtime) dependencies. This is wasteful since most of
the time, we don't need all this cache query.

As it turns out, this is only needed by `bst show` to be able to
figure out the buildable status and the cache key in all circumsances.

This commit makes this behaviour optional, and only enables it from
`bst show`.
@abderrahim abderrahim force-pushed the abderrahim/minimal-cache-query branch from 65b1807 to 90ee532 Compare December 11, 2024 14:14
@gtristan gtristan merged commit 9953897 into master Dec 11, 2024
17 checks passed
@gtristan gtristan deleted the abderrahim/minimal-cache-query branch December 11, 2024 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants