Reviewers: Mads Ager,
Description:
Don't externalize fresh strings.
With the current API the embedder has to extrenalize a string each
time a string is encountered to avoid the cost of repeated character
copying/conversion. The issue here is that the externalization cost
itself is non-negligible (both in time and space) and should not be
paid for a rarely used string. This change is an attempt to predict a
string's usage frequency based on its freshness. A string is
considered fresh if it was recently allocated in the new space.
Please review this at http://codereview.chromium.org/608006
Affected files:
M src/api.cc
M test/cctest/test-api.cc
Index: src/api.cc
diff --git a/src/api.cc b/src/api.cc
index
bd5fdd81dc5274c98e4ad6795957f5c06f1c1d0b..e1fedbe21817df0b62e3a78913337a8b2b36f3ee
100644
--- a/src/api.cc
+++ b/src/api.cc
@@ -3130,12 +3130,24 @@ Local<String> v8::String::NewExternal(
}
+static const int kFreshnessLimit = 1024;
+
+// Determines whether a string is fresh, that is whether it's
+// sufficiently close to the new space top.
+static inline bool IsFreshString(i::Handle<i::String> string) {
+ i::Address address = reinterpret_cast<i::Address>(*string);
+ i::Address top = i::Heap::NewSpaceTop();
+ return top - kFreshnessLimit <= address && address <= top;
+}
+
+
bool v8::String::MakeExternal(
v8::String::ExternalAsciiStringResource* resource) {
if (IsDeadCheck("v8::String::MakeExternal()")) return false;
if (this->IsExternal()) return false; // Already an external string.
ENTER_V8;
i::Handle<i::String> obj = Utils::OpenHandle(this);
+ if (IsFreshString(obj)) return false;
bool result = obj->MakeExternal(resource);
if (result && !obj->IsSymbol()) {
i::ExternalStringTable::AddString(*obj);
@@ -3147,6 +3159,7 @@ bool v8::String::MakeExternal(
bool v8::String::CanMakeExternal() {
if (IsDeadCheck("v8::String::CanMakeExternal()")) return false;
i::Handle<i::String> obj = Utils::OpenHandle(this);
+ if (IsFreshString(obj)) return false;
int size = obj->Size(); // Byte size of the original string.
if (size < i::ExternalString::kSize)
return false;
Index: test/cctest/test-api.cc
diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index
4e617be41fadc183179a1b881f494121d1782a60..41fb02caf1b3585952c9480f4cc27a1be4ffb8c1
100644
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -394,6 +394,9 @@ THREADED_TEST(ScriptMakingExternalString) {
v8::HandleScope scope;
LocalContext env;
Local<String> source = String::New(two_byte_source);
+ // Trigger GCs so that the newly allocated string moves to old gen.
+ i::Heap::CollectGarbage(0, i::NEW_SPACE); // in survivor space now
+ i::Heap::CollectGarbage(0, i::NEW_SPACE); // in old gen now
bool success = source->MakeExternal(new TestResource(two_byte_source));
CHECK(success);
Local<Script> script = Script::Compile(source);
@@ -416,6 +419,9 @@ THREADED_TEST(ScriptMakingExternalAsciiString) {
v8::HandleScope scope;
LocalContext env;
Local<String> source = v8_str(c_source);
+ // Trigger GCs so that the newly allocated string moves to old gen.
+ i::Heap::CollectGarbage(0, i::NEW_SPACE); // in survivor space now
+ i::Heap::CollectGarbage(0, i::NEW_SPACE); // in old gen now
bool success = source->MakeExternal(
new TestAsciiResource(i::StrDup(c_source)));
CHECK(success);
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
