Revision: 24622
Author: [email protected]
Date: Wed Oct 15 10:11:08 2014 UTC
Log: Use WeakCell to handle the script wrapper cache
The script wrapper cache used the API weak handles to provide a weak link
from Script to ScriptWrapper. We want to change the way API weakness works,
and in this context it's best to get rid of users of the API that don't
need to be users.
[email protected]
BUG=
Review URL: https://codereview.chromium.org/659513003
https://code.google.com/p/v8/source/detail?r=24622
Modified:
/branches/bleeding_edge/src/factory.cc
/branches/bleeding_edge/src/objects-inl.h
/branches/bleeding_edge/src/objects.cc
/branches/bleeding_edge/src/objects.h
/branches/bleeding_edge/src/serialize.cc
=======================================
--- /branches/bleeding_edge/src/factory.cc Tue Oct 14 14:43:45 2014 UTC
+++ /branches/bleeding_edge/src/factory.cc Wed Oct 15 10:11:08 2014 UTC
@@ -837,7 +837,6 @@
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 @@
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));
=======================================
--- /branches/bleeding_edge/src/objects-inl.h Tue Oct 14 14:43:45 2014 UTC
+++ /branches/bleeding_edge/src/objects-inl.h Wed Oct 15 10:11:08 2014 UTC
@@ -5407,7 +5407,7 @@
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)
=======================================
--- /branches/bleeding_edge/src/objects.cc Tue Oct 14 14:46:11 2014 UTC
+++ /branches/bleeding_edge/src/objects.cc Wed Oct 15 10:11:08 2014 UTC
@@ -9725,56 +9725,29 @@
}
return result;
}
-
-
-// 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;
}
=======================================
--- /branches/bleeding_edge/src/objects.h Tue Oct 14 14:46:11 2014 UTC
+++ /branches/bleeding_edge/src/objects.h Wed Oct 15 10:11:08 2014 UTC
@@ -860,6 +860,7 @@
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 @@
// [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 @@
// 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)
=======================================
--- /branches/bleeding_edge/src/serialize.cc Mon Oct 13 07:50:21 2014 UTC
+++ /branches/bleeding_edge/src/serialize.cc Wed Oct 15 10:11:08 2014 UTC
@@ -2017,13 +2017,6 @@
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();
- }
-
if (FLAG_trace_code_serializer) {
PrintF("Encoding heap object: ");
heap_object->ShortPrint();
--
--
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.
