Revision: 10744
Author: [email protected]
Date: Mon Feb 20 00:42:18 2012
Log: Avoid sharing AccessorPairs during Genesis.
To test the upcoming changes for map sharing in the presence of accessors,
it is
essential that we keep a few global invariants: The map tree should always
stay
a tree and AccessorPairs should not be shared between different
DescriptorArrays
and/or StringDictionaries. This CL adds a test method for the latter
invariant
and makes some changes to the bootstrapping process to avoid such sharing.
Note that we can't enable the new test method permanently yet, because we
currently go back and forth between fast mode and slow mode when adding an
accessor and break this invariant temporarily. This will be handled in a
separate CL.
Review URL: https://chromiumcodereview.appspot.com/9417043
http://code.google.com/p/v8/source/detail?r=10744
Modified:
/branches/bleeding_edge/src/bootstrapper.cc
/branches/bleeding_edge/src/heap.cc
/branches/bleeding_edge/src/heap.h
=======================================
--- /branches/bleeding_edge/src/bootstrapper.cc Mon Feb 13 06:15:43 2012
+++ /branches/bleeding_edge/src/bootstrapper.cc Mon Feb 20 00:42:18 2012
@@ -172,6 +172,10 @@
Handle<JSFunction> GetThrowTypeErrorFunction();
void CreateStrictModeFunctionMaps(Handle<JSFunction> empty);
+
+ // Make the "arguments" and "caller" properties throw a TypeError on
access.
+ void PoisonArgumentsAndCaller(Handle<Map> map);
+
// Creates the global objects using the global and the template passed in
// through the API. We call this regardless of whether we are building a
// context from scratch or using a deserialized one from the partial
snapshot
@@ -256,14 +260,10 @@
Handle<Map> CreateStrictModeFunctionMap(
PrototypePropertyMode prototype_mode,
- Handle<JSFunction> empty_function,
- Handle<AccessorPair> arguments_callbacks,
- Handle<AccessorPair> caller_callbacks);
+ Handle<JSFunction> empty_function);
Handle<DescriptorArray> ComputeStrictFunctionInstanceDescriptor(
- PrototypePropertyMode propertyMode,
- Handle<AccessorPair> arguments,
- Handle<AccessorPair> caller);
+ PrototypePropertyMode propertyMode);
static bool CompileBuiltin(Isolate* isolate, int index);
static bool CompileExperimentalBuiltin(Isolate* isolate, int index);
@@ -384,44 +384,40 @@
Handle<DescriptorArray> Genesis::ComputeFunctionInstanceDescriptor(
PrototypePropertyMode prototypeMode) {
- Handle<DescriptorArray> descriptors =
- factory()->NewDescriptorArray(prototypeMode == DONT_ADD_PROTOTYPE
- ? 4
- : 5);
- PropertyAttributes attributes =
- static_cast<PropertyAttributes>(DONT_ENUM | DONT_DELETE | READ_ONLY);
+ int size = (prototypeMode == DONT_ADD_PROTOTYPE) ? 4 : 5;
+ Handle<DescriptorArray> descriptors(factory()->NewDescriptorArray(size));
+ PropertyAttributes attribs = static_cast<PropertyAttributes>(
+ DONT_ENUM | DONT_DELETE | READ_ONLY);
DescriptorArray::WhitenessWitness witness(*descriptors);
{ // Add length.
- Handle<Foreign> foreign =
factory()->NewForeign(&Accessors::FunctionLength);
- CallbacksDescriptor d(*factory()->length_symbol(), *foreign,
attributes);
+ Handle<Foreign> f(factory()->NewForeign(&Accessors::FunctionLength));
+ CallbacksDescriptor d(*factory()->length_symbol(), *f, attribs);
descriptors->Set(0, &d, witness);
}
{ // Add name.
- Handle<Foreign> foreign =
factory()->NewForeign(&Accessors::FunctionName);
- CallbacksDescriptor d(*factory()->name_symbol(), *foreign, attributes);
+ Handle<Foreign> f(factory()->NewForeign(&Accessors::FunctionName));
+ CallbacksDescriptor d(*factory()->name_symbol(), *f, attribs);
descriptors->Set(1, &d, witness);
}
{ // Add arguments.
- Handle<Foreign> foreign =
- factory()->NewForeign(&Accessors::FunctionArguments);
- CallbacksDescriptor d(*factory()->arguments_symbol(), *foreign,
attributes);
+ Handle<Foreign>
f(factory()->NewForeign(&Accessors::FunctionArguments));
+ CallbacksDescriptor d(*factory()->arguments_symbol(), *f, attribs);
descriptors->Set(2, &d, witness);
}
{ // Add caller.
- Handle<Foreign> foreign =
factory()->NewForeign(&Accessors::FunctionCaller);
- CallbacksDescriptor d(*factory()->caller_symbol(), *foreign,
attributes);
+ Handle<Foreign> f(factory()->NewForeign(&Accessors::FunctionCaller));
+ CallbacksDescriptor d(*factory()->caller_symbol(), *f, attribs);
descriptors->Set(3, &d, witness);
}
if (prototypeMode != DONT_ADD_PROTOTYPE) {
// Add prototype.
if (prototypeMode == ADD_WRITEABLE_PROTOTYPE) {
- attributes = static_cast<PropertyAttributes>(attributes &
~READ_ONLY);
- }
- Handle<Foreign> foreign =
- factory()->NewForeign(&Accessors::FunctionPrototype);
- CallbacksDescriptor d(*factory()->prototype_symbol(), *foreign,
attributes);
+ attribs = static_cast<PropertyAttributes>(attribs & ~READ_ONLY);
+ }
+ Handle<Foreign>
f(factory()->NewForeign(&Accessors::FunctionPrototype));
+ CallbacksDescriptor d(*factory()->prototype_symbol(), *f, attribs);
descriptors->Set(4, &d, witness);
}
descriptors->Sort(witness);
@@ -532,47 +528,42 @@
Handle<DescriptorArray> Genesis::ComputeStrictFunctionInstanceDescriptor(
- PrototypePropertyMode prototypeMode,
- Handle<AccessorPair> arguments,
- Handle<AccessorPair> caller) {
- Handle<DescriptorArray> descriptors =
- factory()->NewDescriptorArray(prototypeMode == DONT_ADD_PROTOTYPE
- ? 4
- : 5);
- PropertyAttributes attributes = static_cast<PropertyAttributes>(
+ PrototypePropertyMode prototypeMode) {
+ int size = (prototypeMode == DONT_ADD_PROTOTYPE) ? 4 : 5;
+ Handle<DescriptorArray> descriptors(factory()->NewDescriptorArray(size));
+ PropertyAttributes attribs = static_cast<PropertyAttributes>(
DONT_ENUM | DONT_DELETE);
DescriptorArray::WhitenessWitness witness(*descriptors);
- { // length
- Handle<Foreign> foreign =
factory()->NewForeign(&Accessors::FunctionLength);
- CallbacksDescriptor d(*factory()->length_symbol(), *foreign,
attributes);
+ { // Add length.
+ Handle<Foreign> f(factory()->NewForeign(&Accessors::FunctionLength));
+ CallbacksDescriptor d(*factory()->length_symbol(), *f, attribs);
descriptors->Set(0, &d, witness);
}
- { // name
- Handle<Foreign> foreign =
factory()->NewForeign(&Accessors::FunctionName);
- CallbacksDescriptor d(*factory()->name_symbol(), *foreign, attributes);
+ { // Add name.
+ Handle<Foreign> f(factory()->NewForeign(&Accessors::FunctionName));
+ CallbacksDescriptor d(*factory()->name_symbol(), *f, attribs);
descriptors->Set(1, &d, witness);
}
- { // arguments
- CallbacksDescriptor d(*factory()->arguments_symbol(),
- *arguments,
- attributes);
+ { // Add arguments.
+ Handle<AccessorPair> arguments(factory()->NewAccessorPair());
+ CallbacksDescriptor d(*factory()->arguments_symbol(), *arguments,
attribs);
descriptors->Set(2, &d, witness);
}
- { // caller
- CallbacksDescriptor d(*factory()->caller_symbol(), *caller,
attributes);
+ { // Add caller.
+ Handle<AccessorPair> caller(factory()->NewAccessorPair());
+ CallbacksDescriptor d(*factory()->caller_symbol(), *caller, attribs);
descriptors->Set(3, &d, witness);
}
- // prototype
if (prototypeMode != DONT_ADD_PROTOTYPE) {
+ // Add prototype.
if (prototypeMode != ADD_WRITEABLE_PROTOTYPE) {
- attributes = static_cast<PropertyAttributes>(attributes | READ_ONLY);
- }
- Handle<Foreign> foreign =
- factory()->NewForeign(&Accessors::FunctionPrototype);
- CallbacksDescriptor d(*factory()->prototype_symbol(), *foreign,
attributes);
+ attribs = static_cast<PropertyAttributes>(attribs | READ_ONLY);
+ }
+ Handle<Foreign>
f(factory()->NewForeign(&Accessors::FunctionPrototype));
+ CallbacksDescriptor d(*factory()->prototype_symbol(), *f, attribs);
descriptors->Set(4, &d, witness);
}
@@ -603,14 +594,10 @@
Handle<Map> Genesis::CreateStrictModeFunctionMap(
PrototypePropertyMode prototype_mode,
- Handle<JSFunction> empty_function,
- Handle<AccessorPair> arguments_callbacks,
- Handle<AccessorPair> caller_callbacks) {
+ Handle<JSFunction> empty_function) {
Handle<Map> map = factory()->NewMap(JS_FUNCTION_TYPE, JSFunction::kSize);
Handle<DescriptorArray> descriptors =
- ComputeStrictFunctionInstanceDescriptor(prototype_mode,
- arguments_callbacks,
- caller_callbacks);
+ ComputeStrictFunctionInstanceDescriptor(prototype_mode);
map->set_instance_descriptors(*descriptors);
map->set_function_with_prototype(prototype_mode != DONT_ADD_PROTOTYPE);
map->set_prototype(*empty_function);
@@ -619,23 +606,15 @@
void Genesis::CreateStrictModeFunctionMaps(Handle<JSFunction> empty) {
- // Create the callbacks arrays for ThrowTypeError functions.
- // The get/set callacks are filled in after the maps are created below.
- Factory* factory = empty->GetIsolate()->factory();
- Handle<AccessorPair> arguments(factory->NewAccessorPair());
- Handle<AccessorPair> caller(factory->NewAccessorPair());
-
// Allocate map for the strict mode function instances.
Handle<Map> strict_mode_function_instance_map =
- CreateStrictModeFunctionMap(
- ADD_WRITEABLE_PROTOTYPE, empty, arguments, caller);
+ CreateStrictModeFunctionMap(ADD_WRITEABLE_PROTOTYPE, empty);
global_context()->set_strict_mode_function_instance_map(
*strict_mode_function_instance_map);
// Allocate map for the prototype-less strict mode instances.
Handle<Map> strict_mode_function_without_prototype_map =
- CreateStrictModeFunctionMap(
- DONT_ADD_PROTOTYPE, empty, arguments, caller);
+ CreateStrictModeFunctionMap(DONT_ADD_PROTOTYPE, empty);
global_context()->set_strict_mode_function_without_prototype_map(
*strict_mode_function_without_prototype_map);
@@ -643,26 +622,38 @@
// only for processing of builtins.
// Later the map is replaced with writable prototype map, allocated
below.
Handle<Map> strict_mode_function_map =
- CreateStrictModeFunctionMap(
- ADD_READONLY_PROTOTYPE, empty, arguments, caller);
+ CreateStrictModeFunctionMap(ADD_READONLY_PROTOTYPE, empty);
global_context()->set_strict_mode_function_map(
*strict_mode_function_map);
// The final map for the strict mode functions. Writeable prototype.
// This map is installed in MakeFunctionInstancePrototypeWritable.
strict_mode_function_instance_map_writable_prototype_ =
- CreateStrictModeFunctionMap(
- ADD_WRITEABLE_PROTOTYPE, empty, arguments, caller);
-
- // Create the ThrowTypeError function instance.
- Handle<JSFunction> throw_function =
- GetThrowTypeErrorFunction();
+ CreateStrictModeFunctionMap(ADD_WRITEABLE_PROTOTYPE, empty);
// Complete the callbacks.
- arguments->set_getter(*throw_function);
- arguments->set_setter(*throw_function);
- caller->set_getter(*throw_function);
- caller->set_setter(*throw_function);
+ PoisonArgumentsAndCaller(strict_mode_function_instance_map);
+ PoisonArgumentsAndCaller(strict_mode_function_without_prototype_map);
+ PoisonArgumentsAndCaller(strict_mode_function_map);
+ PoisonArgumentsAndCaller(
+ strict_mode_function_instance_map_writable_prototype_);
+}
+
+
+static void SetAccessors(Handle<Map> map,
+ Handle<String> name,
+ Handle<JSFunction> func) {
+ DescriptorArray* descs = map->instance_descriptors();
+ int number = descs->Search(*name);
+ AccessorPair* accessors = AccessorPair::cast(descs->GetValue(number));
+ accessors->set_getter(*func);
+ accessors->set_setter(*func);
+}
+
+
+void Genesis::PoisonArgumentsAndCaller(Handle<Map> map) {
+ SetAccessors(map, factory()->arguments_symbol(),
GetThrowTypeErrorFunction());
+ SetAccessors(map, factory()->caller_symbol(),
GetThrowTypeErrorFunction());
}
=======================================
--- /branches/bleeding_edge/src/heap.cc Wed Feb 15 04:13:55 2012
+++ /branches/bleeding_edge/src/heap.cc Mon Feb 20 00:42:18 2012
@@ -5062,8 +5062,39 @@
cell_space_->Verify(&no_dirty_regions_visitor);
lo_space_->Verify();
+
+ // TODO(svenpanne) We should enable this when our fast->slow->fast-mode
dance
+ // for setting accessor properties is fixed.
+ // VerifyNoAccessorPairSharing();
}
+
+void Heap::VerifyNoAccessorPairSharing() {
+ // Verification is done in 2 phases: First we mark all AccessorPairs,
checking
+ // that we mark only unmarked pairs, then we clear all marks, restoring
the
+ // initial state. We use the Smi tag of the AccessorPair's getter as the
+ // marking bit, because we can never see a Smi as the getter.
+ for (int phase = 0; phase < 2; phase++) {
+ HeapObjectIterator iter(map_space());
+ for (HeapObject* obj = iter.Next(); obj != NULL; obj = iter.Next()) {
+ if (obj->IsMap()) {
+ DescriptorArray* descs = Map::cast(obj)->instance_descriptors();
+ for (int i = 0; i < descs->number_of_descriptors(); i++) {
+ if (descs->GetType(i) == CALLBACKS &&
+ descs->GetValue(i)->IsAccessorPair()) {
+ AccessorPair* accessors =
AccessorPair::cast(descs->GetValue(i));
+ uintptr_t before =
reinterpret_cast<intptr_t>(accessors->getter());
+ uintptr_t after = (phase == 0) ?
+ ((before & ~kSmiTagMask) | kSmiTag) :
+ ((before & ~kHeapObjectTag) | kHeapObjectTag);
+ CHECK(before != after);
+ accessors->set_getter(reinterpret_cast<Object*>(after));
+ }
+ }
+ }
+ }
+ }
+}
#endif // DEBUG
=======================================
--- /branches/bleeding_edge/src/heap.h Fri Feb 10 04:36:05 2012
+++ /branches/bleeding_edge/src/heap.h Mon Feb 20 00:42:18 2012
@@ -1219,6 +1219,10 @@
// Verify the heap is in its normal state before or after a GC.
void Verify();
+ // Verify that AccessorPairs are not shared, i.e. make sure that they
have
+ // exactly one pointer to them.
+ void VerifyNoAccessorPairSharing();
+
void OldPointerSpaceCheckStoreBuffer();
void MapSpaceCheckStoreBuffer();
void LargeObjectSpaceCheckStoreBuffer();
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
