Revision: 4816
Author: [email protected]
Date: Tue Jun 8 02:35:47 2010
Log: Remove the SetExternalStringDiposeCallback API
Changed the disposal of external string resources to call a virtual Dispose
method on the resource. The default inplementation of Dispose deletes the
object and will capture the delete operator matching the new operator used
to allocate the object.
Review URL: http://codereview.chromium.org/2658008
http://code.google.com/p/v8/source/detail?r=4816
Modified:
/branches/bleeding_edge/include/v8.h
/branches/bleeding_edge/src/api.cc
/branches/bleeding_edge/src/heap-inl.h
/branches/bleeding_edge/src/heap.cc
/branches/bleeding_edge/src/heap.h
/branches/bleeding_edge/test/cctest/test-api.cc
=======================================
--- /branches/bleeding_edge/include/v8.h Mon Jun 7 23:20:49 2010
+++ /branches/bleeding_edge/include/v8.h Tue Jun 8 02:35:47 2010
@@ -134,6 +134,7 @@
class Arguments;
class Object;
+class Heap;
class Top;
}
@@ -1037,12 +1038,24 @@
class V8EXPORT ExternalStringResourceBase {
public:
virtual ~ExternalStringResourceBase() {}
+
protected:
ExternalStringResourceBase() {}
+
+ /**
+ * Internally V8 will call this Dispose method when the external string
+ * resource is no longer needed. The default implementation will use
the
+ * delete operator. This method can be overridden in subclasses to
+ * control how allocated external string resources are disposed.
+ */
+ virtual void Dispose() { delete this; }
+
private:
// Disallow copying and assigning.
ExternalStringResourceBase(const ExternalStringResourceBase&);
void operator=(const ExternalStringResourceBase&);
+
+ friend class v8::internal::Heap;
};
/**
@@ -1059,10 +1072,17 @@
* buffer.
*/
virtual ~ExternalStringResource() {}
- /** The string data from the underlying buffer.*/
+
+ /**
+ * The string data from the underlying buffer.
+ */
virtual const uint16_t* data() const = 0;
- /** The length of the string. That is, the number of two-byte
characters.*/
+
+ /**
+ * The length of the string. That is, the number of two-byte
characters.
+ */
virtual size_t length() const = 0;
+
protected:
ExternalStringResource() {}
};
@@ -1134,12 +1154,10 @@
/**
* Creates a new external string using the data defined in the given
* resource. When the external string is no longer live on V8's heap the
- * resource will be disposed. If a disposal callback has been set using
- * SetExternalStringDiposeCallback this callback will be called to
dispose
- * the resource. Otherwise, V8 will dispose the resource using the C++
delete
- * operator. The caller of this function should not otherwise delete or
- * modify the resource. Neither should the underlying buffer be
deallocated
- * or modified except through the destructor of the external string
resource.
+ * resource will be disposed by calling its Dispose method. The caller of
+ * this function should not otherwise delete or modify the resource.
Neither
+ * should the underlying buffer be deallocated or modified except
through the
+ * destructor of the external string resource.
*/
static Local<String> NewExternal(ExternalStringResource* resource);
@@ -1157,12 +1175,10 @@
/**
* Creates a new external string using the ascii data defined in the
given
* resource. When the external string is no longer live on V8's heap the
- * resource will be disposed. If a disposal callback has been set using
- * SetExternalStringDiposeCallback this callback will be called to
dispose
- * the resource. Otherwise, V8 will dispose the resource using the C++
delete
- * operator. The caller of this function should not otherwise delete or
- * modify the resource. Neither should the underlying buffer be
deallocated
- * or modified except through the destructor of the external string
resource.
+ * resource will be disposed by calling its Dispose method. The caller of
+ * this function should not otherwise delete or modify the resource.
Neither
+ * should the underlying buffer be deallocated or modified except
through the
+ * destructor of the external string resource.
*/
static Local<String> NewExternal(ExternalAsciiStringResource* resource);
@@ -1262,10 +1278,6 @@
};
-typedef void (*ExternalStringDiposeCallback)
- (String::ExternalStringResourceBase* resource);
-
-
/**
* A JavaScript number value (ECMA-262, 4.3.20)
*/
@@ -2482,15 +2494,6 @@
*/
static void RemoveMessageListeners(MessageCallback that);
- /**
- * Set a callback to be called when an external string is no longer live
on
- * V8's heap. The resource will no longer be needed by V8 and the
embedder
- * can dispose of if. If this callback is not set V8 will free the
resource
- * using the C++ delete operator.
- */
- static void SetExternalStringDiposeCallback(
- ExternalStringDiposeCallback that);
-
/**
* Sets V8 flags from a string.
*/
=======================================
--- /branches/bleeding_edge/src/api.cc Mon Jun 7 23:20:49 2010
+++ /branches/bleeding_edge/src/api.cc Tue Jun 8 02:35:47 2010
@@ -3696,14 +3696,6 @@
}
}
}
-
-
-void V8::SetExternalStringDiposeCallback(
- ExternalStringDiposeCallback callback) {
- if (IsDeadCheck("v8::V8::SetExternalStringDiposeCallback()"))
- return;
- i::Heap::SetExternalStringDiposeCallback(callback);
-}
void V8::SetCounterFunction(CounterLookupCallback callback) {
=======================================
--- /branches/bleeding_edge/src/heap-inl.h Mon Jun 7 01:49:07 2010
+++ /branches/bleeding_edge/src/heap-inl.h Tue Jun 8 02:35:47 2010
@@ -118,11 +118,9 @@
ExternalString::kResourceOffset -
kHeapObjectTag);
- // Dispose of the C++ object.
- if (external_string_dispose_callback_ != NULL) {
- external_string_dispose_callback_(*resource_addr);
- } else {
- delete *resource_addr;
+ // Dispose of the C++ object if it has not already been disposed.
+ if (*resource_addr != NULL) {
+ (*resource_addr)->Dispose();
}
// Clear the resource pointer in the string.
=======================================
--- /branches/bleeding_edge/src/heap.cc Mon Jun 7 08:39:10 2010
+++ /branches/bleeding_edge/src/heap.cc Tue Jun 8 02:35:47 2010
@@ -98,8 +98,6 @@
// set up by ConfigureHeap otherwise.
int Heap::reserved_semispace_size_ = Heap::max_semispace_size_;
-ExternalStringDiposeCallback Heap::external_string_dispose_callback_ =
NULL;
-
List<Heap::GCPrologueCallbackPair> Heap::gc_prologue_callbacks_;
List<Heap::GCEpilogueCallbackPair> Heap::gc_epilogue_callbacks_;
=======================================
--- /branches/bleeding_edge/src/heap.h Mon Jun 7 08:39:10 2010
+++ /branches/bleeding_edge/src/heap.h Tue Jun 8 02:35:47 2010
@@ -689,11 +689,6 @@
// Utility used with flag gc-greedy.
static bool GarbageCollectionGreedyCheck();
#endif
-
- static void SetExternalStringDiposeCallback(
- ExternalStringDiposeCallback callback) {
- external_string_dispose_callback_ = callback;
- }
static void AddGCPrologueCallback(
GCEpilogueCallback callback, GCType gc_type_filter);
@@ -1143,9 +1138,6 @@
// any string when looked up in properties.
static String* hidden_symbol_;
- static ExternalStringDiposeCallback
- external_string_dispose_callback_;
-
// GC callback function, called before and after mark-compact GC.
// Allocations in the callback function are disallowed.
struct GCPrologueCallbackPair {
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc Mon Jun 7 23:20:49 2010
+++ /branches/bleeding_edge/test/cctest/test-api.cc Tue Jun 8 02:35:47 2010
@@ -612,30 +612,33 @@
}
-static int dispose_count = 0;
-static void DisposeExternalStringCount(
- String::ExternalStringResourceBase* resource) {
- dispose_count++;
-}
-
-
-static void DisposeExternalStringDeleteAndCount(
- String::ExternalStringResourceBase* resource) {
- delete resource;
- dispose_count++;
-}
+class TestAsciiResourceWithDisposeControl: public TestAsciiResource {
+ public:
+ static int dispose_calls;
+
+ TestAsciiResourceWithDisposeControl(const char* data, bool dispose)
+ : TestAsciiResource(data),
+ dispose_(dispose) { }
+
+ void Dispose() {
+ ++dispose_calls;
+ if (dispose_) delete this;
+ }
+ private:
+ bool dispose_;
+};
-TEST(ExternalStringWithResourceDisposeCallback) {
- const char* c_source = "1 + 2 * 3";
-
- // Set an external string collected callback which does not delete the
object.
- v8::V8::SetExternalStringDiposeCallback(DisposeExternalStringCount);
+int TestAsciiResourceWithDisposeControl::dispose_calls = 0;
+
+
+TEST(ExternalStringWithDisposeHandling) {
+ const char* c_source = "1 + 2 * 3";
// Use a stack allocated external string resource allocated object.
- dispose_count = 0;
TestAsciiResource::dispose_count = 0;
- TestAsciiResource res_stack(i::StrDup(c_source));
+ TestAsciiResourceWithDisposeControl::dispose_calls = 0;
+ TestAsciiResourceWithDisposeControl res_stack(i::StrDup(c_source),
false);
{
v8::HandleScope scope;
LocalContext env;
@@ -649,16 +652,14 @@
}
v8::internal::CompilationCache::Clear();
v8::internal::Heap::CollectAllGarbage(false);
- CHECK_EQ(1, dispose_count);
+ CHECK_EQ(1, TestAsciiResourceWithDisposeControl::dispose_calls);
CHECK_EQ(0, TestAsciiResource::dispose_count);
- // Set an external string collected callback which does delete the
object.
-
v8::V8::SetExternalStringDiposeCallback(DisposeExternalStringDeleteAndCount);
-
// Use a heap allocated external string resource allocated object.
- dispose_count = 0;
TestAsciiResource::dispose_count = 0;
- TestAsciiResource* res_heap = new TestAsciiResource(i::StrDup(c_source));
+ TestAsciiResourceWithDisposeControl::dispose_calls = 0;
+ TestAsciiResource* res_heap =
+ new TestAsciiResourceWithDisposeControl(i::StrDup(c_source), true);
{
v8::HandleScope scope;
LocalContext env;
@@ -672,7 +673,7 @@
}
v8::internal::CompilationCache::Clear();
v8::internal::Heap::CollectAllGarbage(false);
- CHECK_EQ(1, dispose_count);
+ CHECK_EQ(1, TestAsciiResourceWithDisposeControl::dispose_calls);
CHECK_EQ(1, TestAsciiResource::dispose_count);
}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
