Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@aam
Copy link
Member

@aam aam commented Jul 17, 2017

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.

@aam aam requested a review from a-siva July 17, 2017 22:26
// during code execution.
uint8_t* platform_kernel_data = (uint8_t*) malloc(platform_kernel.size());
memcpy(platform_kernel_data, platform_kernel.data(), platform_kernel.size());

Copy link
Contributor

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.

Copy link
Member Author

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?

Copy link
Contributor

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.

Copy link
Member Author

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

// 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());
Copy link
Contributor

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

Copy link
Member Author

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());
Copy link
Contributor

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?
Copy link
Contributor

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?

Copy link
Member Author

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.

@aam aam merged commit ba9a525 into flutter:master Jul 24, 2017
@aam aam deleted the flutter-kernel-simple branch February 23, 2018 17:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants