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.

Sign up for GitHub

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

benchmark: try building the addons in napi benchmarks #24309

Closed
wants to merge 1 commit into from
Closed

benchmark: try building the addons in napi benchmarks #24309

Show file tree
Hide file tree
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
23 changes: 23 additions & 0 deletions benchmark/common.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,29 @@

const child_process = require('child_process');
const http_benchmarkers = require('./_http-benchmarkers.js');
const path = require('path');

exports.buildAddon = function(directory) {
const root = path.join(__dirname, '../');
const gyp = path.join(root, 'deps/npm/node_modules/node-gyp/bin/node-gyp');
const python = process.env.PYTHON || 'python';
const ret = child_process.spawnSync(process.execPath, [
gyp,
'rebuild',
`--python=${python}`,
`--directory=${path.join(root, directory)}`,
`--nodedir=${root}`
], { env: { ...process.env, MAKEFLAGS: '-j1' }});
if (ret.error || ret.status !== 0) {
console.error('ERROR', ret.error);
console.error('STATUS', ret.status);
console.error('---- stdout ----');
console.error(ret.stdout.toString());
console.error('---- stderr ----');
console.error(ret.stderr.toString());
throw new Error(`Failed to build ${directory}`);
}
};

exports.buildType = process.features.debug ? 'Debug' : 'Release';

Expand Down
22 changes: 18 additions & 4 deletions benchmark/napi/function_args/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,29 @@ let napi;
try {
v8 = require(`./build/${common.buildType}/binding`);
} catch {
console.error(`${__filename}: V8 Binding failed to load`);
process.exit(0);
try {
common.buildAddon('benchmark/napi/function_args');
// eslint-disable-next-line node-core/no-duplicate-requires
v8 = require(`./build/${common.buildType}/binding`);
} catch (e) {
console.error(`${__filename}: V8 Binding failed to load`);
console.error(e);
process.exit(0);
}
Copy link
Member

Choose a reason for hiding this comment

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

As a suggestion: I would rewrite this whole part to something like:

const v8 = common.buildAddon(
  `./build/${common.buildType}/binding`,
  'benchmark/napi/function_args'
);

In that case the common call just tries to require it first and returns it if possible and tries to build the addon otherwise.
If it fails, we also already have all the necessary information printed due to the console.error part in the common part and in the actual error that is thrown if it fails.

This would reduce the number of code lines significantly.

}

try {
napi = require(`./build/${common.buildType}/napi_binding`);
} catch {
console.error(`${__filename}: NAPI-Binding failed to load`);
process.exit(0);
try {
common.buildAddon('benchmark/napi/function_args');
// eslint-disable-next-line node-core/no-duplicate-requires
napi = require(`./build/${common.buildType}/napi_binding`);
} catch (e) {
console.error(`${__filename}: N-API Binding failed to load`);
console.error(e);
process.exit(0);
}
}

const argsTypes = ['String', 'Number', 'Object', 'Array', 'Typedarray',
Expand Down
26 changes: 20 additions & 6 deletions benchmark/napi/function_call/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,39 @@

const assert = require('assert');
const common = require('../../common.js');

// this fails when we try to open with a different version of node,
// which is quite common for benchmarks. so in that case, just
// abort quietly.

let binding;
try {
var binding = require(`./build/${common.buildType}/binding`);
binding = require(`./build/${common.buildType}/binding`);
} catch {
console.error('misc/function_call.js Binding failed to load');
process.exit(0);
try {
common.buildAddon('benchmark/napi/function_call');
// eslint-disable-next-line node-core/no-duplicate-requires
binding = require(`./build/${common.buildType}/binding`);
} catch (e) {
console.error(`${__filename}: V8 Binding failed to load`);
console.error(e);
process.exit(0);
}
}
const cxx = binding.hello;

let napi_binding;
try {
napi_binding = require(`./build/${common.buildType}/napi_binding`);
} catch {
console.error('misc/function_call/index.js NAPI-Binding failed to load');
process.exit(0);
try {
common.buildAddon('benchmark/misc/function_call');
// eslint-disable-next-line node-core/no-duplicate-requires
napi_binding = require(`./build/${common.buildType}/napi_binding`);
} catch (e) {
console.error(`${__filename}: N-API Binding failed to load`);
console.error(e);
process.exit(0);
}
}
const napi = napi_binding.hello;

Expand Down