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

[macOS] Allow loading of AOT snapshots and instructions from elf bundle #21670

Merged
iskakaushik merged 1 commit into from
Oct 7, 2020
Merged

[macOS] Allow loading of AOT snapshots and instructions from elf bundle #21670

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions shell/platform/darwin/macos/framework/Source/FlutterEngine.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h"

#include <algorithm>
#include <filesystem>
#include <vector>

#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterDartProject_Internal.h"
Expand Down Expand Up @@ -162,6 +163,16 @@ static bool OnAcquireExternalTexture(FlutterEngine* engine,

#pragma mark -

namespace {

struct AotDataDeleter {
void operator()(FlutterEngineAOTData aot_data) { FlutterEngineCollectAOTData(aot_data); }
};

using UniqueAotDataPtr = std::unique_ptr<_FlutterEngineAOTData, AotDataDeleter>;
Copy link
Member

@cbracken cbracken Oct 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't really feel like it's being used as a unique_ptr, but rather just as an RAII wrapper that does some cleanup on destruction. It doesn't look like this actually deletes aot_data after it runs the collect function on it.

See the comment below about passing a non-null data ptr to the create function since it's related to lifetime of this struct.


}

@implementation FlutterEngine {
// The embedding-API-level engine object.
FLUTTER_API_SYMBOL(FlutterEngine) _engine;
Expand All @@ -184,6 +195,9 @@ @implementation FlutterEngine {

// A mapping of textureID to internal FlutterExternalTextureGL adapter.
NSMutableDictionary<NSNumber*, FlutterExternalTextureGL*>* _textures;

// Pointer to the Dart AOT snapshot and instruction data.
UniqueAotDataPtr _aotData;
}

- (instancetype)initWithName:(NSString*)labelPrefix project:(FlutterDartProject*)project {
Expand Down Expand Up @@ -274,6 +288,11 @@ - (BOOL)runWithEntrypoint:(NSString*)entrypoint {
};
flutterArguments.custom_task_runners = &custom_task_runners;

_aotData = [self loadAotData:flutterArguments.assets_path];
if (_aotData) {
flutterArguments.aot_data = _aotData.get();
}

FlutterEngineResult result = FlutterEngineInitialize(
FLUTTER_ENGINE_VERSION, &rendererConfig, &flutterArguments, (__bridge void*)(self), &_engine);
if (result != kSuccess) {
Expand All @@ -293,6 +312,33 @@ - (BOOL)runWithEntrypoint:(NSString*)entrypoint {
return YES;
}

- (UniqueAotDataPtr)loadAotData:(std::string)assetsDir {
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Oct 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the correct ObjC naming is loadAOTData:

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Oct 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

macOS embedder style is to declare and document all private methods at the top of the file in the private interface.

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Oct 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're going to use a std::string (it would be much more idiomatic to pass the NSString* you already have available at the call site), it should be a std::string&.

if (!FlutterEngineRunsAOTCompiledDartCode()) {
return nullptr;
}

std::filesystem::path assetsFsDir(assetsDir);
std::filesystem::path elfFile("app_elf_snapshot.so");
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Oct 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this elf data and not the App.framework? Isn't the latter what the tool is creating and bundling? Or is this an alternate path that's just for the test fixture? (In which case, please comment that here.)

auto fullElfPath = assetsFsDir / elfFile;

if (!std::filesystem::exists(fullElfPath)) {
return nullptr;
}

FlutterEngineAOTDataSource source = {};
source.type = kFlutterEngineAOTDataSourceTypeElfPath;
source.elf_path = fullElfPath.c_str();

FlutterEngineAOTData data = nullptr;
auto result = FlutterEngineCreateAOTData(&source, &data);
Copy link
Member

@cbracken cbracken Oct 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't FlutterEngineCreateAOTData require a non-null data_out param?

Copy link
Member

@chinmaygarde chinmaygarde Oct 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does, but this is a non-null pointer to a pointer variable that will be populated on a successful result. On success, the ownership of that variable is given to a unique_ptr with a custom deleter.

Copy link
Member

@cbracken cbracken Oct 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yep -- lgtm! This seems fine then but someone needs to clean up the struct.

if (result != kSuccess) {
NSLog(@"Failed to load AOT data from: %@", @(fullElfPath.c_str()));
return nullptr;
}

return UniqueAotDataPtr(data);
}

- (void)setViewController:(FlutterViewController*)controller {
_viewController = controller;
_mainOpenGLContext = controller.flutterView.openGLContext;
Expand Down