AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

Remove usage of JSArray in Script object

Browse Source 
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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@3374 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
This commit is contained in:
sgjesse@chromium.org 2009-11-27 14:10:48 +00:00
parent c75ff5e8b5
commit 6d163d9fd0
11 changed files with 43 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
include/v8.h
View File

@ -598,7 +598,7 @@ class V8EXPORT Script {
* 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 @@ class V8EXPORT Context {
* 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();
/**

13
src/accessors.cc
View File

@ -315,14 +315,11 @@ Object* Accessors::ScriptGetLineEnds(Object* object, void*) {
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;
}

4
src/api.cc
View File

@ -451,7 +451,7 @@ void Context::Exit() {
}
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 @@ Local<Value> Script::Id() {
}
void Script::SetData(v8::Handle<Value> data) {
void Script::SetData(v8::Handle<String> data) {
ON_BAILOUT("v8::Script::SetData()", return);
LOG_API("Script::SetData");
{

3
src/factory.cc
View File

@ -188,8 +188,7 @@ Handle<Script> Factory::NewScript(Handle<String> source) {
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));

12
src/handles.cc
View File

@ -429,12 +429,12 @@ Handle<JSValue> GetScriptWrapper(Handle<Script> script) {
// 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 @@ void InitScriptLineEnds(Handle<Script> script) {
}
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 @@ int GetScriptLineNumber(Handle<Script> script, int code_pos) {
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;

24
src/messages.js
View File

@ -238,14 +238,15 @@ function MakeError(type, args) {
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 @@ Script.prototype.lineFromPosition = function(position) {
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 @@ Script.prototype.locationFromPosition = function (position,
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 @@ Script.prototype.sourceSlice = function (opt_from_line, opt_to_line) {
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 @@ Script.prototype.sourceLine = function (opt_line) {
}
// 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);
}

9
src/objects-debug.cc
View File

@ -1116,8 +1116,7 @@ void Script::ScriptVerify() {
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 @@ void Script::ScriptPrint() {
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: ");

3
src/objects-inl.h
View File

@ -2324,8 +2324,7 @@ ACCESSORS(Script, context_data, Object, kContextOffset)
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)

17
src/objects.h
View File

@ -3003,10 +3003,7 @@ class Script: public Struct {
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 @@ class Script: public Struct {
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;

22
test/cctest/test-api.cc
View File

@ -8527,7 +8527,7 @@ TEST(Bug528) {
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 @@ TEST(Bug528) {
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 @@ TEST(Bug528) {
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 @@ TEST(Bug528) {
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
}

8
test/cctest/test-debug.cc
View File

@ -5016,7 +5016,7 @@ TEST(ScriptNameAndData) {
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 @@ TEST(ContextData) {
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 @@ static void ExecuteScriptForContextCheck() {
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));