Skip to content

Do invalidation in Build instead of the asset graph #3984

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

Merged
merged 2 commits into from
May 5, 2025

Conversation

davidmorgan
Copy link
Contributor

@davidmorgan davidmorgan commented Apr 24, 2025

For #3811.

Prior to this PR, invalidation uses a combination of: 1) walk rdeps to mark nodes as "needs building" or "needs checking" in the asset graph; and 2) propagate failures via rdeps in Build and 3) check deps in Build.

With this PR, rdeps are not used: invalidation happens only "in order". State that doesn't need serializing, the progress of the checking the current build, is moved out of the asset graph into the Build class.

Changes to AssetNode:

  • Remove pendingBuildAction.
  • Rename lastKnownDigest -> digest and document how it's used.
  • Remove isFailure, add result which is null for not run, false for failed, true for succeeded
  • Remove wasOutput boolean: presence of digest indicates that it was output, because if there is an output, we immediately compute the digest; add getter checking digest instead
  • Tests pass Digest([]) where a digest is needed to indicate wasOutput but the content doesn't matter.

Copy link

github-actions bot commented Apr 24, 2025

PR Health

Changelog Entry ✔️
Package Changed Files

Changes to files need to be accounted for in their respective changelogs.

@davidmorgan davidmorgan force-pushed the invalidation branch 13 times, most recently from 73ea16a to ea50c3b Compare May 1, 2025 11:17
@davidmorgan davidmorgan force-pushed the invalidation branch 4 times, most recently from 5f80d77 to 99c4580 Compare May 1, 2025 14:45
@davidmorgan davidmorgan marked this pull request as ready for review May 2, 2025 11:52
@davidmorgan davidmorgan requested a review from jensjoha May 2, 2025 11:53
'Build ${renderer.build(forInput, outputs)} because '
'${renderer.id(firstNode.id)} was marked for build.',
'Build ${renderer.build(primaryInput, outputs)} the outputs are '
'optional outputs than have become needed.',
Copy link

Choose a reason for hiding this comment

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

that? Or maybe rephrase entirely?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, done.

@davidmorgan davidmorgan merged commit 17f64cb into dart-lang:master May 5, 2025
17 of 19 checks passed
@davidmorgan davidmorgan deleted the invalidation branch May 5, 2025 09:04
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.

2 participants