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

Sync process recipes to avoid fetching unnecessary dependencies #56

Merged
merged 7 commits into from
Jun 8, 2024

Conversation

kylewlacy
Copy link
Member

See this Zulip discussion for some background context: https://brioche.zulipchat.com/#narrow/stream/440653-general/topic/Why.20so.20slow.3F/near/442990093

When running the snippet to download "hello world" on the brioche.dev landing page (brioche run -r hello_world), it would take a long time to run, because it would end up downloading all of the inputs used to build hello_world (even though these were never used at runtime). This was caused by a split between process and complete_process recipes (complete_process is an implementation detail so that even if the input recipes of a process recipe change, it can still be a cache hit as long as those recipes bake to the same artifacts of another process recipe).

Basically, we were only caching the inner complete_process recipe, which in turn meant that clients would end up trying to bake all of the inputs of the outer process recipe, which is why all the dependencies would be fetched.

This PR changes it so that both process and complete_process get synced to the registry from now on. This should make it so the next time the brioche-packages repo gets synced, it should automatically cache the missing process recipes. However, this won't retroactively fix existing clients, since they would never even check the cache for process recipes.

@kylewlacy kylewlacy merged commit 593af53 into main Jun 8, 2024
12 checks passed
@kylewlacy kylewlacy deleted the sync-improvements branch June 8, 2024 18:12
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.

1 participant