Skip to content

benchmark: add n-api function arguments benchmark suite #21555

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

Closed
kenny-y wants to merge 8 commits into from
Closed

benchmark: add n-api function arguments benchmark suite #21555

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6aa13a1
benchmark: add n-api function arguments benchmark suite
kenny-y Jun 27, 2018
cb758e9
Some fine adjustment based on the review comments in
kenny-y Jun 28, 2018
75e6e42
Fixed wrong calc of typedarray offset (result not in use)
kenny-y Jun 29, 2018
173fa80
Removing compiler warnings
kenny-y Jul 2, 2018
10f3fc5
Use benchmark suite feature to match every engines with every args-types
kenny-y Jul 2, 2018
f10939a
Fixing macro styling issue, trailing backslash alignment.
kenny-y Jul 4, 2018
645f38e
Change the local casual macro to a more formal one.
kenny-y Jul 4, 2018
0f6b238
Use only one param alignment style in the helper macro.
kenny-y Jul 5, 2018
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
Prev Previous commit
Next Next commit
Some fine adjustment based on the review comments in 
#21555
  • Loading branch information
@kenny-y
kenny-y committed Jun 28, 2018
commit cb758e9061567fe8aecd58360ccb80b907cf0ee1
42 changes: 37 additions & 5 deletions benchmark/napi/function_args/binding.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@
#include <node.h>
#include <assert.h>

using v8::Isolate;
using v8::Context;
using v8::Local;
using v8::MaybeLocal;
using v8::Value;
using v8::Number;
using v8::String;
Expand Down Expand Up @@ -42,15 +45,44 @@ void CallWithNumber(const FunctionCallbackInfo<Value>& args) {
}

void CallWithObject(const FunctionCallbackInfo<Value>& args) {
v8::Isolate* isolate = args.GetIsolate();
Isolate* isolate = args.GetIsolate();
Local<Context> context = isolate->GetCurrentContext();

assert(args.Length() == 1 && args[0]->IsObject());
if (args.Length() == 1 && args[0]->IsObject()) {
Copy link
Copy Markdown
Contributor

@gabrielschulhof gabrielschulhof Jun 27, 2018

Choose a reason for hiding this comment

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

I think it might be best to save the result of args.Length() == 1 && args[0]->IsObject() first and then both assert() it and use it as the if-statement condition. For clarity, you could assert(argumentsCorrect && "argument length is 1 and the first argument is an object"), where bool argumentsCorrect = (args.Length() == 1 && args[0]->IsObject());

Local<Object> obj = args[0].As<Object>();
Local<Value> map = obj->Get(String::NewFromUtf8(isolate, "map"));
Local<Value> operand = obj->Get(String::NewFromUtf8(isolate, "operand"));
Local<Value> data = obj->Get(String::NewFromUtf8(isolate, "data"));
Local<Value> reduce = obj->Get(String::NewFromUtf8(isolate, "reduce"));

MaybeLocal<String> map_key = String::NewFromUtf8(isolate,
"map", v8::NewStringType::kNormal);
assert(!map_key.IsEmpty());
MaybeLocal<Value> map_maybe = obj->Get(context,
map_key.ToLocalChecked());
assert(!map_maybe.IsEmpty());
Local<Value> map = map_maybe.ToLocalChecked();

MaybeLocal<String> operand_key = String::NewFromUtf8(isolate,
"operand", v8::NewStringType::kNormal);
assert(!operand_key.IsEmpty());
MaybeLocal<Value> operand_maybe = obj->Get(context,
operand_key.ToLocalChecked());
assert(!operand_maybe.IsEmpty());
Local<Value> operand = operand_maybe.ToLocalChecked();

MaybeLocal<String> data_key = String::NewFromUtf8(isolate,
"data", v8::NewStringType::kNormal);
assert(!data_key.IsEmpty());
MaybeLocal<Value> data_maybe = obj->Get(context,
data_key.ToLocalChecked());
assert(!data_maybe.IsEmpty());
Local<Value> data = data_maybe.ToLocalChecked();

MaybeLocal<String> reduce_key = String::NewFromUtf8(isolate,
"reduce", v8::NewStringType::kNormal);
assert(!reduce_key.IsEmpty());
MaybeLocal<Value> reduce_maybe = obj->Get(context,
reduce_key.ToLocalChecked());
assert(!reduce_maybe.IsEmpty());
Local<Value> reduce = reduce_maybe.ToLocalChecked();
}
}

Expand Down
21 changes: 6 additions & 15 deletions benchmark/napi/function_args/napi_binding.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,29 +94,20 @@ static napi_value CallWithObject(napi_env env, napi_callback_info info) {
status = napi_typeof(env, args[0], types);
assert(status == napi_ok);

assert(types[0] == napi_object);
if (types[0] == napi_object) {
napi_value key;
assert(argc == 1 && types[0] == napi_object);
if (argc == 1 && types[0] == napi_object) {
napi_value value;

status = napi_create_string_utf8(env, "map", strlen("map"), &key);
assert(status == napi_ok);
status = napi_get_property(env, args[0], key, &value);
status = napi_get_named_property(env, args[0], "map", &value);
assert(status == napi_ok);

status = napi_create_string_utf8(env, "operand", strlen("operand"), &key);
assert(status == napi_ok);
status = napi_get_property(env, args[0], key, &value);
status = napi_get_named_property(env, args[0], "operand", &value);
assert(status == napi_ok);

status = napi_create_string_utf8(env, "data", strlen("data"), &key);
assert(status == napi_ok);
status = napi_get_property(env, args[0], key, &value);
status = napi_get_named_property(env, args[0], "data", &value);
assert(status == napi_ok);

status = napi_create_string_utf8(env, "reduce", strlen("reduce"), &key);
assert(status == napi_ok);
status = napi_get_property(env, args[0], key, &value);
status = napi_get_named_property(env, args[0], "reduce", &value);
assert(status == napi_ok);
Copy link
Copy Markdown
Contributor

@gabrielschulhof gabrielschulhof Jun 27, 2018

Choose a reason for hiding this comment

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

You could combine these into napi_get_named_property(). That will internally create the string for you.

}

Expand Down