Reviewers: Michael Starzinger,
Message:
Motivation here was that Rafael was seeing getOwnPropertyNames show up in a
profile and I wondered why it might be slow.
Description:
Speed up non-interceptor case of Object.getOwnPropertyNames
When there are interceptors on an object, it's possible to
end up with duplicate property names. But when all the names
are provided by v8, a collision is not possible, so we can
fast-path that case by not de-duping.
Also added better test coverage for interceptor API.
Please review this at https://codereview.chromium.org/12314081/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/v8natives.js
M test/cctest/test-api.cc
Index: src/v8natives.js
diff --git a/src/v8natives.js b/src/v8natives.js
index
3978e88685156e69ec1660f1ea76d2f8b845b3b9..8473dd15960c18255168d1e8ffc266c84748afa6
100644
--- a/src/v8natives.js
+++ b/src/v8natives.js
@@ -1018,9 +1018,13 @@ function ObjectGetOwnPropertyNames(obj) {
// Get the local element names.
var propertyNames = %GetLocalElementNames(obj);
+ for (var i = 0; i < propertyNames.length; ++i) {
+ propertyNames[i] = %_NumberToString(propertyNames[i]);
+ }
// Get names for indexed interceptor properties.
- if (%GetInterceptorInfo(obj) & 1) {
+ var interceptorInfo = %GetInterceptorInfo(obj);
+ if ((interceptorInfo & 1) != 0) {
var indexedInterceptorNames =
%GetIndexedInterceptorElementNames(obj);
if (indexedInterceptorNames) {
@@ -1034,8 +1038,7 @@ function ObjectGetOwnPropertyNames(obj) {
propertyNames = propertyNames.concat(%GetLocalPropertyNames(obj));
// Get names for named interceptor properties if any.
-
- if (%GetInterceptorInfo(obj) & 2) {
+ if ((interceptorInfo & 2) != 0) {
var namedInterceptorNames =
%GetNamedInterceptorPropertyNames(obj);
if (namedInterceptorNames) {
@@ -1043,21 +1046,24 @@ function ObjectGetOwnPropertyNames(obj) {
}
}
- // Property names are expected to be unique strings.
- var propertySet = { __proto__: null };
- var j = 0;
- for (var i = 0; i < propertyNames.length; ++i) {
- var name = ToString(propertyNames[i]);
- // We need to check for the exact property value since for intrinsic
- // properties like toString if(propertySet["toString"]) will always
- // succeed.
- if (propertySet[name] === true) {
- continue;
+ // Property names are expected to be unique strings,
+ // but interceptors can interfere with that assumption.
+ if (interceptorInfo != 0) {
+ var propertySet = { __proto__: null };
+ var j = 0;
+ for (var i = 0; i < propertyNames.length; ++i) {
+ var name = ToString(propertyNames[i]);
+ // We need to check for the exact property value since for intrinsic
+ // properties like toString if(propertySet["toString"]) will always
+ // succeed.
+ if (propertySet[name] === true) {
+ continue;
+ }
+ propertySet[name] = true;
+ propertyNames[j++] = name;
}
- propertySet[name] = true;
- propertyNames[j++] = name;
+ propertyNames.length = j;
}
- propertyNames.length = j;
return propertyNames;
}
Index: test/cctest/test-api.cc
diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index
7dd9fc7889d8e2806a763b5251f2b4d13a71b2a5..cf9522486cb051913ea84767130046f9551e8362
100644
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -7554,9 +7554,18 @@ THREADED_TEST(AccessControlGetOwnPropertyNames) {
}
+static v8::Handle<v8::Array> IndexedPropertyEnumerator(const
AccessorInfo&) {
+ v8::Handle<v8::Array> result = v8::Array::New(2);
+ result->Set(0, v8::Integer::New(7));
+ result->Set(1, v8::Object::New());
+ return result;
+}
+
+
static v8::Handle<v8::Array> NamedPropertyEnumerator(const AccessorInfo&
info) {
- v8::Handle<v8::Array> result = v8::Array::New(1);
+ v8::Handle<v8::Array> result = v8::Array::New(2);
result->Set(0, v8_str("x"));
+ result->Set(1, v8::Object::New());
return result;
}
@@ -7565,7 +7574,10 @@ THREADED_TEST(GetOwnPropertyNamesWithInterceptor) {
v8::HandleScope handle_scope;
v8::Handle<v8::ObjectTemplate> obj_template = v8::ObjectTemplate::New();
+ obj_template->Set(v8_str("7"), v8::Integer::New(7));
obj_template->Set(v8_str("x"), v8::Integer::New(42));
+ obj_template->SetIndexedPropertyHandler(NULL, NULL, NULL, NULL,
+ IndexedPropertyEnumerator);
obj_template->SetNamedPropertyHandler(NULL, NULL, NULL, NULL,
NamedPropertyEnumerator);
@@ -7573,9 +7585,17 @@ THREADED_TEST(GetOwnPropertyNamesWithInterceptor) {
v8::Handle<v8::Object> global = context->Global();
global->Set(v8_str("object"), obj_template->NewInstance());
- v8::Handle<Value> value =
- CompileRun("Object.getOwnPropertyNames(object).join(',')");
- CHECK_EQ(v8_str("x"), value);
+ v8::Handle<v8::Value> result =
+ CompileRun("Object.getOwnPropertyNames(object)");
+ CHECK(result->IsArray());
+ v8::Handle<v8::Array> result_array = v8::Handle<v8::Array>::Cast(result);
+ CHECK_EQ(3, result_array->Length());
+ CHECK(result_array->Get(0)->IsString());
+ CHECK(result_array->Get(1)->IsString());
+ CHECK(result_array->Get(2)->IsString());
+ CHECK_EQ(v8_str("7"), result_array->Get(0));
+ CHECK_EQ(v8_str("[object Object]"), result_array->Get(1));
+ CHECK_EQ(v8_str("x"), result_array->Get(2));
}
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
