-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Extract snapshotting logic to Snapshotter class #11820
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
Conversation
Extract a Snapshotter class that can be shared between FLX snapshotting, AOT snapshotting, and assembly AOT snapshotting. Allows for better testability of snapshotting logic. * Extracts script snapshotting used in FLX build. * Adds tests for snapshot checksumming, build invalidation/skipping. Remaining work: disentangle + extract AOT snapshotting and Assembly AOT snapshotting logic from build_aot.dart.
| /// Dart snapshot builder. | ||
| /// | ||
| /// Builds Dart snapshots in one of three modes: | ||
| /// * Script snapshot: architecture-independent snapshot of a Dart script core |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems to switch between singular and plural mid-sentence?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed a word! Done.
|
|
||
| // Compute and record checksums. | ||
| try { | ||
| final Set<String> snapshotInputPaths = await readDepfile(dependencies) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I can't add a comment to the appropriate lines in this file)
Maybe add a todo in this file where gen_snapshot is referenced to indicate that this should be using the new snapshotter instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! Done.
goderbauer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| '--isolate_snapshot_data=$isolateSnapshotData', | ||
| '--packages=$packagesPath', | ||
| '--dependencies=$depfilePath', | ||
| '--print_snapshot_sizes', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we remove this --print option? It adds spurious output when running flutter run --release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extract snapshotting logic to Snapshotter class
Extract a Snapshotter class that can be shared between FLX snapshotting,
AOT snapshotting, and assembly AOT snapshotting. Allows for better
testability of snapshotting logic.
Remaining work: disentangle + extract AOT snapshotting and Assembly AOT
snapshotting logic from build_aot.dart.