Skip to content

Commit

Permalink
node: fix erroneously named function call
Browse files Browse the repository at this point in the history
The initial implementation of setPropByIndex() set the value of an Array
by index during development. Though the final form of the function
simply pushes passed values to an array as passed by arguments. Thus the
functions have been renamed to pushValueToArray() and
push_values_to_array_function() respectively.

Also add define for maximum number of arguments should be used before
hitting the limit of performance increase.

Fixes: 494227b "node: improve GetActiveRequests performance"
PR-URL: nodejs#3780
Reviewed-By: Fedor Indutny <fedor@indutny.com>
  • Loading branch information
trevnorris authored and Fishrock123 committed Jan 6, 2016
1 parent 96501e5 commit 8464667
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 7 deletions.
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 @@ -250,6 +255,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
4 changes: 2 additions & 2 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1063,7 +1063,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 @@ -1607,7 +1607,7 @@ 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();
Local<Function> fn = env->push_values_to_array_function();
static const size_t argc = 8;
Local<Value> argv[argc];
size_t i = 0;
Expand Down
4 changes: 2 additions & 2 deletions src/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,9 @@
}

startup.setupProcessObject = function() {
process._setupProcessObject(setPropByIndex);
process._setupProcessObject(pushValueToArray);

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

0 comments on commit 8464667

Please sign in to comment.