From 84646670717f39ff21fbfccde33a81b80f7849aa Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Tue, 10 Nov 2015 02:58:51 -0700 Subject: [PATCH] node: fix erroneously named function call 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: https://github.com/nodejs/node/pull/3780 Reviewed-By: Fedor Indutny --- src/env.h | 12 +++++++++--- src/node.cc | 4 ++-- src/node.js | 4 ++-- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/env.h b/src/env.h index 0c616ddef21400..6151c57069ad7b 100644 --- a/src/env.h +++ b/src/env.h @@ -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) \ @@ -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) \ @@ -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) \ diff --git a/src/node.cc b/src/node.cc index 49cfc8a06053d2..4c08bae04bdeef 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1063,7 +1063,7 @@ void SetupProcessObject(const FunctionCallbackInfo& args) { CHECK(args[0]->IsFunction()); - env->set_add_properties_by_index_function(args[0].As()); + env->set_push_values_to_array_function(args[0].As()); env->process_object()->Delete( FIXED_ONE_BYTE_STRING(env->isolate(), "_setupProcessObject")); } @@ -1607,7 +1607,7 @@ static void GetActiveRequests(const FunctionCallbackInfo& args) { Local ary = Array::New(args.GetIsolate()); Local ctx = env->context(); - Local fn = env->add_properties_by_index_function(); + Local fn = env->push_values_to_array_function(); static const size_t argc = 8; Local argv[argc]; size_t i = 0; diff --git a/src/node.js b/src/node.js index 34aae9395442a0..8d77adc27c4102 100644 --- a/src/node.js +++ b/src/node.js @@ -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]); }