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

chore(android): update V8 to 7.3.492.26 #10817

Merged
merged 3 commits into from
Apr 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,11 @@ android/.idea/modules.xml
android/.idea/scopes/scope_settings.xml
android/.idea/vcs.xml
*.iml

/iphone/TitaniumKit/Carthage
/iphone/TitaniumKit/docs
/iphone/TitaniumKit/build
/iphone/lib/libTiCore.a

.vscode/
android/runtime/v8/src/native/V8Snapshots.h
6 changes: 1 addition & 5 deletions android/build.xml
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
<project name="titanium-mobile" default="build">
<property name="ti.android.root" location="${basedir}"/>

<import file="build/common.xml"/>

<target name="full.build" depends="clean,build">
</target>

<target name="full.build" depends="clean,build" />
<target name="clean" depends="clean.all" />

<target name="build" depends="build.all" />
</project>
2 changes: 1 addition & 1 deletion android/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
],
"architectures": ["arm64-v8a", "armeabi-v7a", "x86"],
"v8": {
"version": "7.0.276.42",
"version": "7.3.492.26",
"mode": "release"
},
"minSDKVersion": "19",
Expand Down
4 changes: 4 additions & 0 deletions android/runtime/v8/src/native/V8Runtime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "V8Util.h"

#include "V8Runtime.h"
#include "V8Snapshots.h"

#define TAG "V8Runtime"

Expand Down Expand Up @@ -225,6 +226,9 @@ JNIEXPORT void JNICALL Java_org_appcelerator_kroll_runtime_v8_V8Runtime_nativeIn
// Create a new Isolate and make it the current one.
Isolate::CreateParams create_params;
create_params.array_buffer_allocator = &allocator;
#ifdef V8_SNAPSHOT_H
create_params.snapshot_blob = &snapshot;
#endif
isolate = Isolate::New(create_params);
isolate->Enter();

Expand Down
1 change: 1 addition & 0 deletions android/runtime/v8/src/native/V8Snapshots.h
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
// GENERATED AT BUILD TIME
24 changes: 24 additions & 0 deletions build/lib/android/V8Snapshots.h.ejs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#ifndef V8_SNAPSHOT_H
#define V8_SNAPSHOT_H

#include <v8.h>

<% function DEFINE_SNAPSHOT (target, blob) { -%>
<% if (!blob) return -%>
#ifdef __<%- target -%>__
static const unsigned char snapshot_data[] = {
<% for (let i = 0; i < blob.length - 1; i++) { -%>
<%- blob.readUInt8(i) + ',' -%>
<% } -%>
<%- blob.readUInt8(blob.length - 1) %>
};
#endif
<% } -%>
<% DEFINE_SNAPSHOT('i386', x86) -%>
<% DEFINE_SNAPSHOT('arm', arm) -%>
<% DEFINE_SNAPSHOT('aarch64', arm64) -%>

static const int snapshot_size = sizeof(snapshot_data);
static v8::StartupData snapshot = { (const char*) snapshot_data, snapshot_size };

#endif
34 changes: 21 additions & 13 deletions build/lib/android/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const path = require('path');
const fs = require('fs-extra');
const AndroidSDK = require('./androidsdk');
const ant = require('./ant');
const utils = require('../utils');
const copyFile = utils.copyFile;
Expand All @@ -27,25 +28,32 @@ class Android {
this.apiLevel = options.apiLevel;
this.sdkVersion = options.sdkVersion;
this.gitHash = options.gitHash;
this.sdk = new AndroidSDK(this.androidSDK, this.apiLevel);
this.antProperties = {
'build.version': this.sdkVersion,
'build.githash': this.gitHash,
'android.sdk': this.sdk.getAndroidSDK(),
'android.platform': this.sdk.getPlatformDir(),
'google.apis': this.sdk.getGoogleApisDir(),
'kroll.v8.build.x86': 1,
'android.ndk': this.androidNDK
};
}

async clean() {
return ant.build(ANDROID_BUILD_XML, [ 'clean' ], {});
return ant.build(ANDROID_BUILD_XML, [ 'clean' ], this.antProperties);
}

async build() {
const AndroidSDK = require('./androidsdk');
const sdk = new AndroidSDK(this.androidSDK, this.apiLevel);
const properties = {
'build.version': this.sdkVersion,
'build.githash': this.gitHash,
'android.sdk': sdk.getAndroidSDK(),
'android.platform': sdk.getPlatformDir(),
'google.apis': sdk.getGoogleApisDir(),
'kroll.v8.build.x86': 1,
'android.ndk': this.androidNDK
};
return ant.build(ANDROID_BUILD_XML, [ 'full.build' ], properties);
// Clean build and download V8
await this.clean().catch(error => console.error(error));
Copy link
Contributor

Choose a reason for hiding this comment

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

really don't need the catch chained here, it'll bubble up the Error if there is one. adding a catch will actually just log it and move on.


// Generate snapshots
const snapshot = require('./snapshot');
await snapshot.build().catch(error => console.error(error));
Copy link
Contributor

Choose a reason for hiding this comment

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

same here, no need for a catch really.


// Build Titanium Android
return ant.build(ANDROID_BUILD_XML, [ 'build' ], this.antProperties);
}

async package(packager) {
Expand Down
110 changes: 110 additions & 0 deletions build/lib/android/snapshot.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
'use strict';

const spawn = require('child_process').spawn; // eslint-disable-line security/detect-child-process
const os = require('os');
const fs = require('fs-extra');
const path = require('path');
const ejs = require('ejs');

const ANDROID_DIR = path.resolve('..', 'android');
const ANDROID_PROPS = require(path.join(ANDROID_DIR, 'package.json')); // eslint-disable-line security/detect-non-literal-require
const V8_PROPS = ANDROID_PROPS.v8;
const V8_LIB_DIR = path.join('..', 'dist', 'android', 'libv8', V8_PROPS.version, V8_PROPS.mode, 'libs');

// Obtain target architectures
const TARGETS = [];
for (const target of ANDROID_PROPS.architectures) {
if (target === 'arm64-v8a') {
TARGETS.push('arm64');
} else if (target === 'armeabi-v7a') {
TARGETS.push('arm');
} else {
TARGETS.push(target);
}
}

/**
* Runs mksnapshot to generate snapshots for each architecture
* @param {string} target targets to generate blob
* @returns {Promise<void>}
*/
async function generateBlob(target) {
const V8_LIB_TARGET_DIR = path.resolve(V8_LIB_DIR, target);
const MKSNAPSHOT_PATH = path.join(V8_LIB_TARGET_DIR, 'mksnapshot');
const BLOB_PATH = path.join(V8_LIB_TARGET_DIR, 'blob.bin');
const args = [
'--startup_blob=' + BLOB_PATH
];

// Set correct permissions for 'mksnapshot'
fs.chmodSync(MKSNAPSHOT_PATH, 0o755);
Copy link
Contributor

Choose a reason for hiding this comment

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

the method is async, so you could do await fs.chmod(MKSNAPSHOT_PATH, 0o755);


return new Promise((resolve, reject) => {

// Snapshot already exists, skip...
if (fs.existsSync(BLOB_PATH)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this could be moved out of the Promise and use await fs.exists(BLOB_PATH)

resolve();
}

// Generate snapshot
const p = spawn(MKSNAPSHOT_PATH, args);
p.on('close', code => {
if (code !== 0) {
return reject(`"mksnapshot ${args.join(' ')}" failed with exit code: ${code}`);
}
resolve();
});
});
}

/**
* Generates V8Snapshots.h header from architecture blobs
* @returns {Promise<void>}
*/
async function generateHeader() {
const blobs = {};

// Load snapshots for each architecture
for (const target of TARGETS) {
Copy link
Contributor

Choose a reason for hiding this comment

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

again this method is async, so you can use the async fs methods.

Additionally, you could use probably refactor this very slightly to read in all the blobs in parallel by using Promise.all(TARGETS.map());

const V8_LIB_TARGET_DIR = path.resolve(V8_LIB_DIR, target);
const BLOB_PATH = path.join(V8_LIB_TARGET_DIR, 'blob.bin');

if (fs.existsSync(BLOB_PATH)) {
blobs[target] = Buffer.from(fs.readFileSync(BLOB_PATH, 'binary'), 'binary');
}
}

return new Promise(async (resolve, reject) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm confused by the async here...

Generally I try to make use of util.promisify to handle older callback style APIs so I can avoid the hand-written Promise instances. so in this case you could do:

const promisify = require('util').promisify; // at the top of the file
const renderFile = promisify(require('ejs').renderFile);
// ...
const output = await renderFile(path.join(__dirname, 'V8Snapshots.h.ejs'), blobs, {});
return fs.writeFile(path.join(ANDROID_DIR, 'runtime', 'v8', 'src', 'native', 'V8Snapshots.h'), output);


// Generate 'V8Snapshots.h' from template
ejs.renderFile(path.join(__dirname, 'V8Snapshots.h.ejs'), blobs, {}, (error, output) => {
if (error) {
return reject(error);
}
fs.writeFileSync(path.join(ANDROID_DIR, 'runtime', 'v8', 'src', 'native', 'V8Snapshots.h'), output);
resolve();
});
});
}

/**
* Generates empty snapshot blobs for each supported architecture
* and creates a header to include the snapshots at build time.
* NOTE: SNAPSHOT GENERATION IS ONLY SUPPORTED ON macOS
* @returns {Promise<void>}
*/
async function build() {
return new Promise(async (resolve, reject) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

again, if the method is async, you shouldn't need to hand write a Promise instance:

// Only macOS is supports creating snapshots
if (os.platform() !== 'darwin') {
    return;
}

// generate snapshots in parallel
await Promise.all(TARGETS.map(t => generateBlob(t)));
return generateHeader();


// Only macOS is supports creating snapshots
if (os.platform() === 'darwin') {
for (const target of TARGETS) {
await generateBlob(target).catch(error => reject(error));
}
await generateHeader().catch(error => reject(error));
}
resolve();
});
}

module.exports = { build };