Reviewers: ulan,
Description:
Use WeakMap to handle the script wrapper cache
[email protected]
BUG=
Please review this at https://codereview.chromium.org/659513003/
Base URL: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+23, -50 lines):
M src/factory.cc
M src/objects.h
M src/objects.cc
M src/objects-inl.h
M src/serialize.cc
Index: src/factory.cc
diff --git a/src/factory.cc b/src/factory.cc
index
5b196d3ca0aa12c03abb39cab0b220e8448d39bd..d1deb7519a23b6cddce5e2fa2d1ddeab7f1fd9e4
100644
--- a/src/factory.cc
+++ b/src/factory.cc
@@ -837,7 +837,6 @@ Handle<Script> Factory::NewScript(Handle<String>
source) {
heap->set_last_script_id(Smi::FromInt(id));
// Create and initialize script object.
- Handle<Foreign> wrapper = NewForeign(0, TENURED);
Handle<Script> script = Handle<Script>::cast(NewStruct(SCRIPT_TYPE));
script->set_source(*source);
script->set_name(heap->undefined_value());
@@ -846,7 +845,7 @@ Handle<Script> Factory::NewScript(Handle<String>
source) {
script->set_column_offset(Smi::FromInt(0));
script->set_context_data(heap->undefined_value());
script->set_type(Smi::FromInt(Script::TYPE_NORMAL));
- script->set_wrapper(*wrapper);
+ script->set_wrapper(heap->undefined_value());
script->set_line_ends(heap->undefined_value());
script->set_eval_from_shared(heap->undefined_value());
script->set_eval_from_instructions_offset(Smi::FromInt(0));
Index: src/objects-inl.h
diff --git a/src/objects-inl.h b/src/objects-inl.h
index
6385c587c5837b635ce1fc2fbd5f062bb5011d41..91bd0092244536b8293999e80640e56f9235a762
100644
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -5407,7 +5407,7 @@ ACCESSORS(Script, id, Smi, kIdOffset)
ACCESSORS_TO_SMI(Script, line_offset, kLineOffsetOffset)
ACCESSORS_TO_SMI(Script, column_offset, kColumnOffsetOffset)
ACCESSORS(Script, context_data, Object, kContextOffset)
-ACCESSORS(Script, wrapper, Foreign, kWrapperOffset)
+ACCESSORS(Script, wrapper, HeapObject, kWrapperOffset)
ACCESSORS_TO_SMI(Script, type, kTypeOffset)
ACCESSORS(Script, line_ends, Object, kLineEndsOffset)
ACCESSORS(Script, eval_from_shared, Object, kEvalFromSharedOffset)
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index
4f23ea58b9f1e0ef70d7f6b38fcb5ba2e6be085a..300b5b7dddfc3740ef5eff084f991ed1afe154a4
100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -9727,54 +9727,27 @@ Handle<Object>
Script::GetNameOrSourceURL(Handle<Script> script) {
}
-// Wrappers for scripts are kept alive and cached in weak global
-// handles referred from foreign objects held by the scripts as long as
-// they are used. When they are not used anymore, the garbage
-// collector will call the weak callback on the global handle
-// associated with the wrapper and get rid of both the wrapper and the
-// handle.
-static void ClearWrapperCacheWeakCallback(
- const v8::WeakCallbackData<v8::Value, void>& data) {
- Object** location = reinterpret_cast<Object**>(data.GetParameter());
- JSValue* wrapper = JSValue::cast(*location);
- Script::cast(wrapper->value())->ClearWrapperCache();
-}
-
-
-void Script::ClearWrapperCache() {
- Foreign* foreign = wrapper();
- Object** location =
reinterpret_cast<Object**>(foreign->foreign_address());
- DCHECK_EQ(foreign->foreign_address(),
reinterpret_cast<Address>(location));
- foreign->set_foreign_address(0);
- GlobalHandles::Destroy(location);
- GetIsolate()->counters()->script_wrappers()->Decrement();
-}
-
-
Handle<JSObject> Script::GetWrapper(Handle<Script> script) {
- if (script->wrapper()->foreign_address() != NULL) {
- // Return a handle for the existing script wrapper from the cache.
- return Handle<JSValue>(
-
*reinterpret_cast<JSValue**>(script->wrapper()->foreign_address()));
- }
Isolate* isolate = script->GetIsolate();
+ if (!script->wrapper()->IsUndefined()) {
+ Handle<WeakCell> cell(WeakCell::cast(script->wrapper()));
+ if (!cell->value()->IsUndefined()) {
+ // Return a handle for the existing script wrapper from the cache.
+ return handle(JSObject::cast(cell->value()));
+ }
+ // If we found an empty WeakCell, that means the script wrapper was
+ // GCed. We are not notified directly of that, so we decrement here
+ // so that we at least don't count double for any given script.
+ isolate->counters()->script_wrappers()->Decrement();
+ }
// Construct a new script wrapper.
isolate->counters()->script_wrappers()->Increment();
Handle<JSFunction> constructor = isolate->script_function();
Handle<JSValue> result =
Handle<JSValue>::cast(isolate->factory()->NewJSObject(constructor));
-
result->set_value(*script);
-
- // Create a new weak global handle and use it to cache the wrapper
- // for future use. The cache will automatically be cleared by the
- // garbage collector when it is not used anymore.
- Handle<Object> handle = isolate->global_handles()->Create(*result);
- GlobalHandles::MakeWeak(handle.location(),
- reinterpret_cast<void*>(handle.location()),
- &ClearWrapperCacheWeakCallback);
- script->wrapper()->set_foreign_address(
- reinterpret_cast<Address>(handle.location()));
+ Handle<WeakCell> cell = isolate->factory()->NewWeakCell(result);
+ script->set_wrapper(*cell);
return result;
}
Index: src/objects.h
diff --git a/src/objects.h b/src/objects.h
index
69375570ad6a825a3ffb9ddc0e07dcdea57f9f38..5a1830408ec562076eef8aa08e60a97bb1440dfa
100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -860,6 +860,7 @@ class ObjectVisitor;
class LookupIterator;
class StringStream;
class TypeFeedbackVector;
+class WeakCell;
// We cannot just say "class HeapType;" if it is created from a
template... =8-?
template<class> class TypeImpl;
struct HeapTypeConfig;
@@ -6445,8 +6446,8 @@ class Script: public Struct {
// [context_data]: context data for the context this script was compiled
in.
DECL_ACCESSORS(context_data, Object)
- // [wrapper]: the wrapper cache.
- DECL_ACCESSORS(wrapper, Foreign)
+ // [wrapper]: the wrapper cache. This is either undefined or a WeakCell.
+ DECL_ACCESSORS(wrapper, HeapObject)
// [type]: the script type.
DECL_ACCESSORS(type, Smi)
@@ -6508,7 +6509,6 @@ class Script: public Struct {
// Get the JS object wrapping the given script; create it if none exists.
static Handle<JSObject> GetWrapper(Handle<Script> script);
- void ClearWrapperCache();
// Dispatched behavior.
DECLARE_PRINTER(Script)
Index: src/serialize.cc
diff --git a/src/serialize.cc b/src/serialize.cc
index
70b69c23c0dc01beeda05d4bcb2532584df4b5a6..8657ea38502ac185d66741f7816ecf12299ab90e
100644
--- a/src/serialize.cc
+++ b/src/serialize.cc
@@ -2018,10 +2018,11 @@ void
CodeSerializer::SerializeHeapObject(HeapObject* heap_object,
HowToCode how_to_code,
WhereToPoint where_to_point) {
if (heap_object->IsScript()) {
- // The wrapper cache uses a Foreign object to point to a global handle.
- // However, the object visitor expects foreign objects to point to
external
- // references. Clear the cache to avoid this issue.
- Script::cast(heap_object)->ClearWrapperCache();
+ // The wrapper cache uses a WeakCell. However it's not clear what
+ // the semantics of WeakCell are when serializing. For now we just
+ // clear the cache.
+ HeapObject* clear = heap_object->GetHeap()->undefined_value();
+ Script::cast(heap_object)->set_wrapper(clear);
}
if (FLAG_trace_code_serializer) {
--
--
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/d/optout.
