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

Drop Usage of Object::Set() #3780

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 22 additions & 0 deletions benchmark/fs/bench-readdir.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
'use strict';

const common = require('../common');
const fs = require('fs');

const bench = common.createBenchmark(main, {
n: [1e4],
});


function main(conf) {
const n = conf.n >>> 0;

bench.start();
(function r(cntr) {
if (--cntr <= 0)
return bench.end(n);
fs.readdir(__dirname + '/../../lib/', function() {
r(cntr);
});
}(n));
}
19 changes: 19 additions & 0 deletions benchmark/fs/bench-readdirSync.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
'use strict';

const common = require('../common');
const fs = require('fs');

const bench = common.createBenchmark(main, {
n: [1e4],
});


function main(conf) {
const n = conf.n >>> 0;

bench.start();
for (var i = 0; i < n; i++) {
fs.readdirSync(__dirname + '/../../lib/');
}
bench.end(n);
}
55 changes: 55 additions & 0 deletions benchmark/http/bench-parser.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
'use strict';

const common = require('../common');
const HTTPParser = process.binding('http_parser').HTTPParser;
const REQUEST = HTTPParser.REQUEST;
const kOnHeaders = HTTPParser.kOnHeaders | 0;
const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;
const kOnBody = HTTPParser.kOnBody | 0;
const kOnMessageComplete = HTTPParser.kOnMessageComplete | 0;
const CRLF = '\r\n';

const bench = common.createBenchmark(main, {
fields: [4, 8, 16, 32],
n: [1e5],
});


function main(conf) {
const fields = conf.fields >>> 0;
const n = conf.n >>> 0;
var header = `GET /hello HTTP/1.1${CRLF}Content-Type: text/plain${CRLF}`;

for (var i = 0; i < fields; i++) {
header += `X-Filler${i}: ${Math.random().toString(36).substr(2)}${CRLF}`;
}
header += CRLF;

processHeader(new Buffer(header), n);
}


function processHeader(header, n) {
const parser = newParser(REQUEST);

bench.start();
for (var i = 0; i < n; i++) {
parser.execute(header, 0, header.length);
parser.reinitialize(REQUEST);
}
bench.end(n);
}


function newParser(type) {
const parser = new HTTPParser(type);

parser.headers = [];

parser[kOnHeaders] = function() { };
parser[kOnHeadersComplete] = function() { };
parser[kOnBody] = function() { };
parser[kOnMessageComplete] = function() { };

return parser;
}
18 changes: 18 additions & 0 deletions benchmark/misc/bench-env.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
'use strict';

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

const bench = common.createBenchmark(main, {
n: [1e5],
});


function main(conf) {
const n = conf.n >>> 0;
bench.start();
for (var i = 0; i < n; i++) {
// Access every item in object to process values.
Object.keys(process.env);
}
bench.end(n);
}
18 changes: 18 additions & 0 deletions benchmark/misc/bench-hrtime.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
'use strict';

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

const bench = common.createBenchmark(main, {
n: [1e6]
});


function main(conf) {
const n = conf.n >>> 0;

bench.start();
for (var i = 0; i < n; i++) {
process.hrtime();
}
bench.end(n);
}
12 changes: 9 additions & 3 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ namespace node {
#define NODE_ISOLATE_SLOT 3
#endif

// The number of items passed to push_values_to_array_function has diminishing
// returns around 8. This should be used at all call sites using said function.
#ifndef NODE_PUSH_VAL_TO_ARRAY_MAX
#define NODE_PUSH_VAL_TO_ARRAY_MAX 8
#endif

// Strings are per-isolate primitives but Environment proxies them
// for the sake of convenience. Strings should be ASCII-only.
#define PER_ISOLATE_STRING_PROPERTIES(V) \
Expand Down Expand Up @@ -231,12 +237,11 @@ namespace node {
V(zero_return_string, "ZERO_RETURN") \

#define ENVIRONMENT_STRONG_PERSISTENT_PROPERTIES(V) \
V(add_properties_by_index_function, v8::Function) \
V(as_external, v8::External) \
V(async_hooks_destroy_function, v8::Function) \
V(async_hooks_init_function, v8::Function) \
V(async_hooks_pre_function, v8::Function) \
V(async_hooks_post_function, v8::Function) \
V(async_hooks_destroy_function, v8::Function) \
V(async_hooks_pre_function, v8::Function) \
V(binding_cache_object, v8::Object) \
V(buffer_constructor_function, v8::Function) \
V(buffer_prototype_object, v8::Object) \
Expand All @@ -249,6 +254,7 @@ namespace node {
V(pipe_constructor_template, v8::FunctionTemplate) \
V(process_object, v8::Object) \
V(promise_reject_function, v8::Function) \
V(push_values_to_array_function, v8::Function) \
V(script_context_constructor_template, v8::FunctionTemplate) \
V(script_data_constructor_function, v8::Function) \
V(secure_context_constructor_template, v8::FunctionTemplate) \
Expand Down
112 changes: 67 additions & 45 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1027,7 +1027,7 @@ void SetupProcessObject(const FunctionCallbackInfo<Value>& args) {

CHECK(args[0]->IsFunction());

env->set_add_properties_by_index_function(args[0].As<Function>());
env->set_push_values_to_array_function(args[0].As<Function>());
env->process_object()->Delete(
FIXED_ONE_BYTE_STRING(env->isolate(), "_setupProcessObject"));
}
Expand Down Expand Up @@ -1571,28 +1571,22 @@ static void GetActiveRequests(const FunctionCallbackInfo<Value>& args) {

Local<Array> ary = Array::New(args.GetIsolate());
Local<Context> ctx = env->context();
Local<Function> fn = env->add_properties_by_index_function();
static const size_t argc = 8;
Local<Value> argv[argc];
size_t i = 0;
Local<Function> fn = env->push_values_to_array_function();
Local<Value> argv[NODE_PUSH_VAL_TO_ARRAY_MAX];
size_t idx = 0;

for (auto w : *env->req_wrap_queue()) {
if (w->persistent().IsEmpty() == false) {
argv[i++ % argc] = w->object();
if ((i % argc) == 0) {
HandleScope scope(env->isolate());
fn->Call(ctx, ary, argc, argv).ToLocalChecked();
for (auto&& arg : argv) {
arg = Local<Value>();
}
}
if (w->persistent().IsEmpty())
continue;
argv[idx] = w->object();
if (++idx >= ARRAY_SIZE(argv)) {
fn->Call(ctx, ary, idx, argv).ToLocalChecked();
idx = 0;
}
}

const size_t remainder = i % argc;
if (remainder > 0) {
HandleScope scope(env->isolate());
fn->Call(ctx, ary, remainder, argv).ToLocalChecked();
if (idx > 0) {
fn->Call(ctx, ary, idx, argv).ToLocalChecked();
}

args.GetReturnValue().Set(ary);
Expand All @@ -1605,7 +1599,10 @@ void GetActiveHandles(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

Local<Array> ary = Array::New(env->isolate());
int i = 0;
Local<Context> ctx = env->context();
Local<Function> fn = env->push_values_to_array_function();
Local<Value> argv[NODE_PUSH_VAL_TO_ARRAY_MAX];
size_t idx = 0;

Local<String> owner_sym = env->owner_string();

Expand All @@ -1616,7 +1613,14 @@ void GetActiveHandles(const FunctionCallbackInfo<Value>& args) {
Local<Value> owner = object->Get(owner_sym);
if (owner->IsUndefined())
owner = object;
ary->Set(i++, owner);
argv[idx] = owner;
if (++idx >= ARRAY_SIZE(argv)) {
fn->Call(ctx, ary, idx, argv).ToLocalChecked();
idx = 0;
}
}
if (idx > 0) {
fn->Call(ctx, ary, idx, argv).ToLocalChecked();
}

args.GetReturnValue().Set(ary);
Expand Down Expand Up @@ -2092,22 +2096,23 @@ void Hrtime(const FunctionCallbackInfo<Value>& args) {

uint64_t t = uv_hrtime();

if (args.Length() > 0) {
// return a time diff tuple
if (!args[0]->IsArray()) {
if (!args[1]->IsUndefined()) {
if (!args[1]->IsArray()) {
return env->ThrowTypeError(
"process.hrtime() only accepts an Array tuple.");
"process.hrtime() only accepts an Array tuple");
}
Local<Array> inArray = Local<Array>::Cast(args[0]);
uint64_t seconds = inArray->Get(0)->Uint32Value();
uint64_t nanos = inArray->Get(1)->Uint32Value();
t -= (seconds * NANOS_PER_SEC) + nanos;
args.GetReturnValue().Set(true);
}

Local<Array> tuple = Array::New(env->isolate(), 2);
tuple->Set(0, Integer::NewFromUnsigned(env->isolate(), t / NANOS_PER_SEC));
tuple->Set(1, Integer::NewFromUnsigned(env->isolate(), t % NANOS_PER_SEC));
args.GetReturnValue().Set(tuple);
Local<ArrayBuffer> ab = args[0].As<Uint32Array>()->Buffer();
uint32_t* fields = static_cast<uint32_t*>(ab->GetContents().Data());

// These three indices will contain the values for the hrtime tuple. The
// seconds value is broken into the upper/lower 32 bits and stored in two
// uint32 fields to be converted back in JS.
fields[0] = (t / NANOS_PER_SEC) >> 32;
fields[1] = (t / NANOS_PER_SEC) & 0xffffffff;
fields[2] = t % NANOS_PER_SEC;
}

extern "C" void node_module_register(void* m) {
Expand Down Expand Up @@ -2519,31 +2524,42 @@ static void EnvDeleter(Local<String> property,


static void EnvEnumerator(const PropertyCallbackInfo<Array>& info) {
Isolate* isolate = info.GetIsolate();
Environment* env = Environment::GetCurrent(info);
Isolate* isolate = env->isolate();
Local<Context> ctx = env->context();
Local<Function> fn = env->push_values_to_array_function();
Local<Value> argv[NODE_PUSH_VAL_TO_ARRAY_MAX];
size_t idx = 0;

#ifdef __POSIX__
int size = 0;
while (environ[size])
size++;

Local<Array> envarr = Array::New(isolate, size);
Local<Array> envarr = Array::New(isolate);

for (int i = 0; i < size; ++i) {
const char* var = environ[i];
const char* s = strchr(var, '=');
const int length = s ? s - var : strlen(var);
Local<String> name = String::NewFromUtf8(isolate,
var,
String::kNormalString,
length);
envarr->Set(i, name);
argv[idx] = String::NewFromUtf8(isolate,
var,
String::kNormalString,
length);
if (++idx >= ARRAY_SIZE(argv)) {
fn->Call(ctx, envarr, idx, argv).ToLocalChecked();
idx = 0;
}
}
if (idx > 0) {
fn->Call(ctx, envarr, idx, argv).ToLocalChecked();
}
#else // _WIN32
WCHAR* environment = GetEnvironmentStringsW();
if (environment == nullptr)
return; // This should not happen.
Local<Array> envarr = Array::New(isolate);
WCHAR* p = environment;
int i = 0;
while (*p) {
WCHAR *s;
if (*p == L'=') {
Expand All @@ -2558,13 +2574,19 @@ static void EnvEnumerator(const PropertyCallbackInfo<Array>& info) {
}
const uint16_t* two_byte_buffer = reinterpret_cast<const uint16_t*>(p);
const size_t two_byte_buffer_len = s - p;
Local<String> value = String::NewFromTwoByte(isolate,
two_byte_buffer,
String::kNormalString,
two_byte_buffer_len);
envarr->Set(i++, value);
argv[idx] = String::NewFromTwoByte(isolate,
two_byte_buffer,
String::kNormalString,
two_byte_buffer_len);
if (++idx >= ARRAY_SIZE(argv)) {
fn->Call(ctx, envarr, idx, argv).ToLocalChecked();
idx = 0;
}
p = s + wcslen(s) + 1;
}
if (idx > 0) {
fn->Call(ctx, envarr, idx, argv).ToLocalChecked();
}
FreeEnvironmentStringsW(environment);
#endif

Expand Down
19 changes: 17 additions & 2 deletions src/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,12 +178,27 @@
}

startup.setupProcessObject = function() {
process._setupProcessObject(setPropByIndex);
const _hrtime = process.hrtime;
const hrValues = new Uint32Array(3);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a short comment here or in hrtime() explaining what the fields contain after an update.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added to node::Hrtime().


function setPropByIndex() {
process._setupProcessObject(pushValueToArray);

function pushValueToArray() {
for (var i = 0; i < arguments.length; i++)
this.push(arguments[i]);
}

process.hrtime = function hrtime(ar) {
const ret = [0, 0];
if (_hrtime(hrValues, ar)) {
ret[0] = (hrValues[0] * 0x100000000 + hrValues[1]) - ar[0];
ret[1] = hrValues[2] - ar[1];
} else {
ret[0] = hrValues[0] * 0x100000000 + hrValues[1];
ret[1] = hrValues[2];
}
return ret;
};
};

startup.globalVariables = function() {
Expand Down
Loading