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

[rush] Refactor build cache to reduce critical path length #4275

Closed
wants to merge 3 commits into from

Conversation

dmichon-msft
Copy link
Contributor

Summary

Refactors the legacy skip logic and the build cache to each be their own PhasedOperationPlugin.

Schedules reading from and writing to the build cache as separate operations in the queue.
This allows for cache reads to happen earlier in the pipeline, and allows for cache writes to occur in parallel with operations that consume the output.

The skip detection logic and build cache will now never coexist in the same execution.

Details

Added new hooks to PhasedOperationHooks:

  • beforeExecuteOperation - Executed immediately prior to the operation executing. If this returns an OperationStatus, the operation will be skipped, and the status will be the result of the operation.
  • afterExecuteOperation - Executed immediately following the operation's execution. If this returns an OperationStatus, it will override the existing status from execution. This hook does not get called if beforeExecuteOperation returned a value.

Skip detection is implemented by tapping beforeExecuteOperation and injecting a check if the operation can be skipped. It also taps afterExecuteOperation to propagate whether or not skipping is allowed.

The build cache taps createOperations and injects additional synthetic operations before (if incremental builds are enabled) and after (if cache writes are enabled) each normal operation in the graph. It uses custom runners for cache read and write that communicate with taps on the beforeExecuteOperation and afterExecuteOperation hooks.

With this change, if a cache read fails, the build will log a warning and proceed to try executing the operation normally. If a cache write fails, consumers of the operation being written to the cache will not be blocked, though the build will still log a failure at the end.

Adds a new parameter to the callback in Async.forEachAsync to allow deferral of a currently executing task. The expected use case for this feature is to support #3949, wherein Rush needs to be able to defer execution while waiting for the results of a remotely executing operation.

How it was tested

Local invocation in both incremental and non-incremental mode.
Unit tests for the Async.forEachAsync code.

Impacted documentation

Any documentation about how cache reads/writes are scheduled.

The hooks for the build cache.

Moves the build cache to its own plugin.
Moves skip detection to its own plugin.
Creates additional operations in the graph for cache reads/writes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

1 participant