Skip to content

Lazily compute outputs #3960

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 1 commit into from
Apr 11, 2025
Merged

Conversation

davidmorgan
Copy link
Contributor

@davidmorgan davidmorgan commented Apr 4, 2025

For #3811.

"outputs" are used at the start of the build to know what to invalidate/remove when updating the graph. Since they're not used during the build, they can be computed in one go instead of incrementally. It's not clear if there is a benefit to serializing them--stop doing that for now.

Benchmarks improve quite a bit due to no longer doing many repeated copies of the immutable "outputs" sets, and the JSON size is about halved.

before this PR
json_serializable
shape,libraries,clean/ms,no changes/ms,incremental/ms,json/KiB
loop,1,21805,3912,5216,454
loop,100,24872,4058,7354,5002
loop,250,27289,4901,11688,27067
loop,500,41936,7753,16928,104465
loop,750,76779,12352,23427,232645
loop,1000,133677,18266,37324,419464
forwards,1,21847,3851,5081,455
forwards,100,24577,3687,7520,3169
forwards,250,28194,4798,10244,15451
forwards,500,34165,5963,16175,57796
forwards,750,47009,8607,22132,127484
forwards,1000,70189,10267,26040,228472
backwards,1,22138,3970,5189,455
backwards,100,24881,4113,7513,3213
backwards,250,28388,4596,11579,15708
backwards,500,34640,5917,17745,58798
backwards,750,51449,8806,25058,129720
backwards,1000,75859,12535,32259,232430

with this PR
json_serializable
shape,libraries,clean/ms,no changes/ms,incremental/ms,json/KiB
loop,1,23492,3887,5292,417
loop,100,23786,3900,6928,2564
loop,250,27987,4494,9667,12674
loop,500,32715,5789,16602,47884
loop,750,37391,8292,22735,106042
loop,1000,43723,11432,30665,191073
forwards,1,22246,3762,5264,417
forwards,100,24092,3960,7265,1741
forwards,250,27831,4231,9800,7467
forwards,500,29865,5024,14991,26973
forwards,750,36458,6234,20061,58929
forwards,1000,39756,8070,28756,105307
backwards,1,22162,3845,5364,417
backwards,100,24036,4054,7900,1762
backwards,250,27503,4349,10938,7594
backwards,500,31306,5024,15695,27471
backwards,750,37975,6045,22651,60042
backwards,1000,43777,8021,27442,107280

Copy link

github-actions bot commented Apr 4, 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 remove-outputs branch 6 times, most recently from f4b439f to d1cfd70 Compare April 11, 2025 09:49
@davidmorgan davidmorgan marked this pull request as ready for review April 11, 2025 10:19
@davidmorgan davidmorgan requested a review from jensjoha April 11, 2025 10:19
@davidmorgan
Copy link
Contributor Author

The CI issue is due to _macros package being removed in the latest dev SDK, unrelated to this change: kicked off a separate discussion for that.

@davidmorgan davidmorgan merged commit a5f7a85 into dart-lang:master Apr 11, 2025
74 of 75 checks passed
@davidmorgan davidmorgan deleted the remove-outputs branch April 11, 2025 11:14
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