Revision: 3374 Author: [email protected] Date: Fri Nov 27 06:10:48 2009 Log: Remove usage of JSArray in Script object
Storing a JSArray in the Script object could cause an indirect reference from the compilation cache to a global object to be created. Now the line ends are only stored as a FixedArrya and when that is needed in JavaScript a JSArray copy is created. Changed some of the JavaScript code to cache the line ends in a local variable for better performance. BUG=http://code.google.com/p/v8/issues/detail?id=528 TEST=test/test-api/Bug528 Review URL: http://codereview.chromium.org/434117 http://code.google.com/p/v8/source/detail?r=3374 Modified: /branches/bleeding_edge/include/v8.h /branches/bleeding_edge/src/accessors.cc /branches/bleeding_edge/src/api.cc /branches/bleeding_edge/src/factory.cc /branches/bleeding_edge/src/handles.cc /branches/bleeding_edge/src/messages.js /branches/bleeding_edge/src/objects-debug.cc /branches/bleeding_edge/src/objects-inl.h /branches/bleeding_edge/src/objects.h /branches/bleeding_edge/test/cctest/test-api.cc /branches/bleeding_edge/test/cctest/test-debug.cc ======================================= --- /branches/bleeding_edge/include/v8.h Tue Nov 24 06:10:06 2009 +++ /branches/bleeding_edge/include/v8.h Fri Nov 27 06:10:48 2009 @@ -598,7 +598,7 @@ * with the debugger as this data object is only available through the * debugger API. */ - void SetData(Handle<Value> data); + void SetData(Handle<String> data); }; @@ -2634,7 +2634,7 @@ * with the debugger to provide additional information on the context through * the debugger API. */ - void SetData(Handle<Value> data); + void SetData(Handle<String> data); Local<Value> GetData(); /** ======================================= --- /branches/bleeding_edge/src/accessors.cc Wed Nov 11 01:19:39 2009 +++ /branches/bleeding_edge/src/accessors.cc Fri Nov 27 06:10:48 2009 @@ -315,14 +315,11 @@ HandleScope scope; Handle<Script> script(Script::cast(JSValue::cast(object)->value())); InitScriptLineEnds(script); - if (script->line_ends_js_array()->IsUndefined()) { - Handle<FixedArray> line_ends_fixed_array( - FixedArray::cast(script->line_ends_fixed_array())); - Handle<FixedArray> copy = Factory::CopyFixedArray(line_ends_fixed_array); - Handle<JSArray> js_array = Factory::NewJSArrayWithElements(copy); - script->set_line_ends_js_array(*js_array); - } - return script->line_ends_js_array(); + ASSERT(script->line_ends()->IsFixedArray()); + Handle<FixedArray> line_ends(FixedArray::cast(script->line_ends())); + Handle<FixedArray> copy = Factory::CopyFixedArray(line_ends); + Handle<JSArray> js_array = Factory::NewJSArrayWithElements(copy); + return *js_array; } ======================================= --- /branches/bleeding_edge/src/api.cc Wed Nov 18 00:59:28 2009 +++ /branches/bleeding_edge/src/api.cc Fri Nov 27 06:10:48 2009 @@ -451,7 +451,7 @@ } -void Context::SetData(v8::Handle<Value> data) { +void Context::SetData(v8::Handle<String> data) { if (IsDeadCheck("v8::Context::SetData()")) return; ENTER_V8; { @@ -1175,7 +1175,7 @@ } -void Script::SetData(v8::Handle<Value> data) { +void Script::SetData(v8::Handle<String> data) { ON_BAILOUT("v8::Script::SetData()", return); LOG_API("Script::SetData"); { ======================================= --- /branches/bleeding_edge/src/factory.cc Wed Nov 11 01:19:39 2009 +++ /branches/bleeding_edge/src/factory.cc Fri Nov 27 06:10:48 2009 @@ -188,8 +188,7 @@ script->set_type(Smi::FromInt(Script::TYPE_NORMAL)); script->set_compilation_type(Smi::FromInt(Script::COMPILATION_TYPE_HOST)); script->set_wrapper(*wrapper); - script->set_line_ends_fixed_array(Heap::undefined_value()); - script->set_line_ends_js_array(Heap::undefined_value()); + script->set_line_ends(Heap::undefined_value()); script->set_eval_from_function(Heap::undefined_value()); script->set_eval_from_instructions_offset(Smi::FromInt(0)); ======================================= --- /branches/bleeding_edge/src/handles.cc Tue Nov 17 16:09:28 2009 +++ /branches/bleeding_edge/src/handles.cc Fri Nov 27 06:10:48 2009 @@ -429,12 +429,12 @@ // Init line_ends array with code positions of line ends inside script // source. void InitScriptLineEnds(Handle<Script> script) { - if (!script->line_ends_fixed_array()->IsUndefined()) return; + if (!script->line_ends()->IsUndefined()) return; if (!script->source()->IsString()) { ASSERT(script->source()->IsUndefined()); - script->set_line_ends_fixed_array(*(Factory::NewFixedArray(0))); - ASSERT(script->line_ends_fixed_array()->IsFixedArray()); + script->set_line_ends(*(Factory::NewFixedArray(0))); + ASSERT(script->line_ends()->IsFixedArray()); return; } @@ -467,8 +467,8 @@ } ASSERT(array_index == line_count); - script->set_line_ends_fixed_array(*array); - ASSERT(script->line_ends_fixed_array()->IsFixedArray()); + script->set_line_ends(*array); + ASSERT(script->line_ends()->IsFixedArray()); } @@ -477,7 +477,7 @@ InitScriptLineEnds(script); AssertNoAllocation no_allocation; FixedArray* line_ends_array = - FixedArray::cast(script->line_ends_fixed_array()); + FixedArray::cast(script->line_ends()); const int line_ends_len = line_ends_array->length(); int line = -1; ======================================= --- /branches/bleeding_edge/src/messages.js Fri Oct 23 02:19:17 2009 +++ /branches/bleeding_edge/src/messages.js Fri Nov 27 06:10:48 2009 @@ -238,14 +238,15 @@ Script.prototype.lineFromPosition = function(position) { var lower = 0; var upper = this.lineCount() - 1; + var line_ends = this.line_ends; // We'll never find invalid positions so bail right away. - if (position > this.line_ends[upper]) { + if (position > line_ends[upper]) { return -1; } // This means we don't have to safe-guard indexing line_ends[i - 1]. - if (position <= this.line_ends[0]) { + if (position <= line_ends[0]) { return 0; } @@ -253,9 +254,9 @@ while (upper >= 1) { var i = (lower + upper) >> 1; - if (position > this.line_ends[i]) { + if (position > line_ends[i]) { lower = i + 1; - } else if (position <= this.line_ends[i - 1]) { + } else if (position <= line_ends[i - 1]) { upper = i - 1; } else { return i; @@ -278,8 +279,9 @@ if (line == -1) return null; // Determine start, end and column. - var start = line == 0 ? 0 : this.line_ends[line - 1] + 1; - var end = this.line_ends[line]; + var line_ends = this.line_ends; + var start = line == 0 ? 0 : line_ends[line - 1] + 1; + var end = line_ends[line]; if (end > 0 && StringCharAt.call(this.source, end - 1) == '\r') end--; var column = position - start; @@ -368,8 +370,9 @@ return null; } - var from_position = from_line == 0 ? 0 : this.line_ends[from_line - 1] + 1; - var to_position = to_line == 0 ? 0 : this.line_ends[to_line - 1] + 1; + var line_ends = this.line_ends; + var from_position = from_line == 0 ? 0 : line_ends[from_line - 1] + 1; + var to_position = to_line == 0 ? 0 : line_ends[to_line - 1] + 1; // Return a source slice with line numbers re-adjusted to the resource. return new SourceSlice(this, from_line + this.line_offset, to_line + this.line_offset, @@ -391,8 +394,9 @@ } // Return the source line. - var start = line == 0 ? 0 : this.line_ends[line - 1] + 1; - var end = this.line_ends[line]; + var line_ends = this.line_ends; + var start = line == 0 ? 0 : line_ends[line - 1] + 1; + var end = line_ends[line]; return StringSubstring.call(this.source, start, end); } ======================================= --- /branches/bleeding_edge/src/objects-debug.cc Thu Nov 26 23:57:45 2009 +++ /branches/bleeding_edge/src/objects-debug.cc Fri Nov 27 06:10:48 2009 @@ -1116,8 +1116,7 @@ VerifyPointer(data()); VerifyPointer(wrapper()); type()->SmiVerify(); - VerifyPointer(line_ends_fixed_array()); - VerifyPointer(line_ends_js_array()); + VerifyPointer(line_ends()); VerifyPointer(id()); } @@ -1144,10 +1143,8 @@ wrapper()->ShortPrint(); PrintF("\n - compilation type: "); compilation_type()->ShortPrint(); - PrintF("\n - line ends fixed array: "); - line_ends_fixed_array()->ShortPrint(); - PrintF("\n - line ends js array: "); - line_ends_js_array()->ShortPrint(); + PrintF("\n - line ends: "); + line_ends()->ShortPrint(); PrintF("\n - eval from function: "); eval_from_function()->ShortPrint(); PrintF("\n - eval from instructions offset: "); ======================================= --- /branches/bleeding_edge/src/objects-inl.h Wed Nov 25 04:55:33 2009 +++ /branches/bleeding_edge/src/objects-inl.h Fri Nov 27 06:10:48 2009 @@ -2324,8 +2324,7 @@ ACCESSORS(Script, wrapper, Proxy, kWrapperOffset) ACCESSORS(Script, type, Smi, kTypeOffset) ACCESSORS(Script, compilation_type, Smi, kCompilationTypeOffset) -ACCESSORS(Script, line_ends_fixed_array, Object, kLineEndsFixedArrayOffset) -ACCESSORS(Script, line_ends_js_array, Object, kLineEndsJSArrayOffset) +ACCESSORS(Script, line_ends, Object, kLineEndsOffset) ACCESSORS(Script, eval_from_function, Object, kEvalFromFunctionOffset) ACCESSORS(Script, eval_from_instructions_offset, Smi, kEvalFrominstructionsOffsetOffset) ======================================= --- /branches/bleeding_edge/src/objects.h Wed Nov 25 04:55:33 2009 +++ /branches/bleeding_edge/src/objects.h Fri Nov 27 06:10:48 2009 @@ -3003,10 +3003,7 @@ DECL_ACCESSORS(compilation_type, Smi) // [line_ends]: FixedArray of line ends positions. - DECL_ACCESSORS(line_ends_fixed_array, Object) - - // [line_ends]: JSArray of line ends positions. - DECL_ACCESSORS(line_ends_js_array, Object) + DECL_ACCESSORS(line_ends, Object) // [eval_from_function]: for eval scripts the funcion from which eval was // called. @@ -3036,16 +3033,8 @@ static const int kWrapperOffset = kContextOffset + kPointerSize; static const int kTypeOffset = kWrapperOffset + kPointerSize; static const int kCompilationTypeOffset = kTypeOffset + kPointerSize; - // We have the line ends array both in FixedArray form and in JSArray form. - // The FixedArray form is useful when we don't have a context and so can't - // create a JSArray. The JSArray form is useful when we want to see the - // array from JS code (e.g. debug-delay.js) which cannot handle unboxed - // FixedArray objects. - static const int kLineEndsFixedArrayOffset = - kCompilationTypeOffset + kPointerSize; - static const int kLineEndsJSArrayOffset = - kLineEndsFixedArrayOffset + kPointerSize; - static const int kIdOffset = kLineEndsJSArrayOffset + kPointerSize; + static const int kLineEndsOffset = kCompilationTypeOffset + kPointerSize; + static const int kIdOffset = kLineEndsOffset + kPointerSize; static const int kEvalFromFunctionOffset = kIdOffset + kPointerSize; static const int kEvalFrominstructionsOffsetOffset = kEvalFromFunctionOffset + kPointerSize; ======================================= --- /branches/bleeding_edge/test/cctest/test-api.cc Fri Nov 27 00:19:25 2009 +++ /branches/bleeding_edge/test/cctest/test-api.cc Fri Nov 27 06:10:48 2009 @@ -8527,7 +8527,7 @@ v8::HandleScope scope; context->Enter(); - Local<v8::Object> obj = v8::Object::New(); + Local<v8::String> obj = v8::String::New(""); context->SetData(obj); CompileRun("1"); context->Exit(); @@ -8538,13 +8538,7 @@ if (GetGlobalObjectsCount() == 0) break; } CHECK_EQ(0, GetGlobalObjectsCount()); - - // Compilation cache size is different for Android. -#if defined(ANDROID) - CHECK_EQ(1, gc_count); -#else - CHECK_EQ(5, gc_count); -#endif + CHECK_EQ(2, gc_count); // Eval in a function creates reference from the compilation cache to the // global object. @@ -8562,13 +8556,7 @@ if (GetGlobalObjectsCount() == 0) break; } CHECK_EQ(0, GetGlobalObjectsCount()); - - // Compilation cache size is different for Android. -#if defined(ANDROID) - CHECK_EQ(1, gc_count); -#else CHECK_EQ(2, gc_count); -#endif // Looking up the line number for an exception creates reference from the // compilation cache to the global object. @@ -8591,11 +8579,5 @@ if (GetGlobalObjectsCount() == 0) break; } CHECK_EQ(0, GetGlobalObjectsCount()); - - // Compilation cache size is different for Android. -#if defined(ANDROID) CHECK_EQ(2, gc_count); -#else - CHECK_EQ(5, gc_count); -#endif -} +} ======================================= --- /branches/bleeding_edge/test/cctest/test-debug.cc Wed Nov 18 00:59:28 2009 +++ /branches/bleeding_edge/test/cctest/test-debug.cc Fri Nov 27 06:10:48 2009 @@ -5016,7 +5016,7 @@ v8::ScriptOrigin origin2 = v8::ScriptOrigin(v8::String::New("new name")); v8::Handle<v8::Script> script2 = v8::Script::Compile(script, &origin2); script2->Run(); - script2->SetData(data_obj); + script2->SetData(data_obj->ToString()); f = v8::Local<v8::Function>::Cast(env->Global()->Get(v8::String::New("f"))); f->Call(env->Global(), 0, NULL); CHECK_EQ(3, break_point_hit_count); @@ -5069,8 +5069,8 @@ CHECK(context_2->GetData()->IsUndefined()); // Set and check different data values. - v8::Handle<v8::Value> data_1 = v8::Number::New(1); - v8::Handle<v8::Value> data_2 = v8::String::New("2"); + v8::Handle<v8::String> data_1 = v8::String::New("1"); + v8::Handle<v8::String> data_2 = v8::String::New("2"); context_1->SetData(data_1); context_2->SetData(data_2); CHECK(context_1->GetData()->StrictEquals(data_1)); @@ -5233,7 +5233,7 @@ CHECK(context_1->GetData()->IsUndefined()); // Set and check a data value. - v8::Handle<v8::Value> data_1 = v8::Number::New(1); + v8::Handle<v8::String> data_1 = v8::String::New("1"); context_1->SetData(data_1); CHECK(context_1->GetData()->StrictEquals(data_1)); -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
