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

Use built_value for serialization. #3915

Merged
merged 1 commit into from
Mar 13, 2025

Conversation

davidmorgan
Copy link
Contributor

@davidmorgan davidmorgan commented Mar 10, 2025

For #3811.

Remove hand-coded serialization of AssetNode, use built_value's generated code.

Store globs as their String pattern instead of as a Glob to save having to serialize the Glob type.

Before this PR the serialization was dropping half of the graph data on serialization and reconstructing it on deserialization, adding back inputs by reversing the outputs links on load. Stop doing that and just serialize+deserialize unchanged. This turns out to need one fix for correctness: track one input in build_impl.dart that was being restored on load.

The output is quite a lot larger than before, for the 1000 node benchmark the serialized graph is now 228Mb(!) whereas previously it was 20Mb(!). This is because the previous serialization code optimizes by replacing AssetIds with one int each used to refer to that ID throughout the JSON. The 10x saving is nice, but we need to "dedupe" whole subgraphs, not just individual IDs, in the end the graph for that benchmark should be well under 1Mb.

This restores a good chunk of the performance lost in the previous PR, the details are not important as we are obviously some way from the desired final state, but here are the numbers in case anyone wants them

json_serializable
shape,libraries,clean/ms,no changes/ms,incremental/ms
loop,1,22922,4294,5286
loop,100,24904,4455,8344
loop,250,30565,5193,13646
loop,500,53779,7798,25373
loop,750,105386,12212,45280
loop,1000,187782,20752,74371

Copy link

github-actions bot commented Mar 10, 2025

PR Health

@davidmorgan davidmorgan force-pushed the serialize-built branch 2 times, most recently from 9b59494 to 7b06b1f Compare March 11, 2025 08:38
@davidmorgan davidmorgan force-pushed the serialize-built branch 2 times, most recently from 8055e50 to 04828a2 Compare March 11, 2025 16:06
@davidmorgan davidmorgan marked this pull request as ready for review March 11, 2025 16:32
@davidmorgan davidmorgan requested a review from jensjoha March 11, 2025 16:32
@davidmorgan davidmorgan merged commit 501bfa8 into dart-lang:master Mar 13, 2025
83 of 84 checks passed
@davidmorgan davidmorgan deleted the serialize-built branch March 13, 2025 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants