-
Notifications
You must be signed in to change notification settings - Fork 6k
Update Flutter engine kernel-loading logic. #3886
Conversation
| // during code execution. | ||
| uint8_t* platform_kernel_data = (uint8_t*) malloc(platform_kernel.size()); | ||
| memcpy(platform_kernel_data, platform_kernel.data(), platform_kernel.size()); | ||
|
|
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.
maybe we should add fields platform_kernel_data and platform_kernel_size to the DartController class and do this copy in the constructor of DartController, that way we would be able to reuse it from there for every call to CreateIsolateFor instead of doing the memcpy everytime. Also we can free the buffer in the destructor of DartController.
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.
We won't know platform_kernel in DartController constructor, we will only know it in CreateIsolateFor. So we won't be able to copy it in constructor, will we?
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.
Agree, I was wondering about the multiple copies being created for each isolate until I realized the service isolate is still being started the old way. If we switched the service isolate to also use the kernel platform file then you may have to save this once and not keep re-reading this everytime.
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.
To build service isolate from kernel we would have to embed platform.dill into engine similarly to how it is done for vm snapshots https://github.com/flutter/engine/blob/master/shell/common/engine.cc#L129, it seems
runtime/dart_controller.cc
Outdated
| // This is needed because original kernel data has to be available | ||
| // during code execution. | ||
| uint8_t* kernel_data = (uint8_t*) malloc(kernel.size()); | ||
| memcpy(kernel_data, kernel.data(), kernel.size()); |
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.
When do we free these copies
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.
Fixed. Now I am keeping pointers to those bytes in dart_controller instance and free them when controller is destroyed.
| zip_asset_store->GetAsBuffer(kPlatformKernelAssetKey, &platform_data); | ||
| if (!platform_data.empty()) { | ||
| kernel_platform = | ||
| Dart_ReadKernelBinary(platform_data.data(), platform_data.size()); |
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.
FTL_DCHECK(kernel_platform != NULL);
| } | ||
|
|
||
| std::string entry_uri = script_uri; | ||
| // Are we running from a Dart source file? |
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.
The service isolate is still being started from source, the old way? It has not been migrated to using the kernel platform file?
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.
No, not yet. It might still be helpful to start with using kernel only for the flutter app first, then add loading of service isolate from kernel.
Update Flutter engine kernel-loading logic so it loads core libraries from platform.dill.
platform.dill is expected to come as part of application .flx archive. As before "loading from kernel" logic is triggered if .flx archive has "kernel_blob.bin" file(which is main script .dill-file).
This changes are to support https://github.com/aam/flutter/tree/flutter-tools-kernel-mode changes to support Dart frontend by flutter tools.