Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(21)

Unified Diff: src/api.cc

Issue 608006: Don't externalize fresh strings. (Closed)
Patch Set: Tracking of string usage. Created 10 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | test/cctest/test-api.cc » ('j') | test/cctest/test-api.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/api.cc
diff --git a/src/api.cc b/src/api.cc
index bd5fdd81dc5274c98e4ad6795957f5c06f1c1d0b..c7b645e767d0788ccc1ac3e8e2e30015a6274247 100644
--- a/src/api.cc
+++ b/src/api.cc
@@ -2458,6 +2458,69 @@ Handle<Value> Function::GetName() const {
}
+namespace {
+
+// Tracks string usage to help make better decisions when
+// externalizing strings.
+class StringTracker {
+ public:
+ // Records that the given string's characters were copied to some
Mads Ager (chromium) 2010/02/16 08:16:19 Could you update the comments in this class to mak
Vitaly Repeshko 2010/02/16 18:56:42 Done.
+ // external buffer. If this happens often we should honor
+ // externalization requests for the string.
+ static void RecordWrite(i::Handle<i::String> string) {
+ i::Address address = reinterpret_cast<i::Address>(*string);
+ i::Address top = i::Heap::NewSpaceTop();
+ if (IsFreshString(address, top)) {
+ IncrementUseCount(top);
+ }
+ }
+
+ // Estimates freshness and use frequency of the given string based
+ // on how close it is to the new space top and its recorded usage
+ // history.
+ static inline bool IsFreshUnusedString(i::Handle<i::String> string) {
+ i::Address address = reinterpret_cast<i::Address>(*string);
+ i::Address top = i::Heap::NewSpaceTop();
+ return IsFreshString(address, top) && IsUseCountLow(top);
+ }
+
+ private:
+ static inline bool IsFreshString(i::Address string, i::Address top) {
+ return top - kFreshnessLimit <= string && string <= top;
+ }
+
+ static inline bool IsUseCountLow(i::Address top) {
+ if (last_top_ != top) return true;
+ return use_count_ < kUseLimit;
+ }
+
+ static inline void IncrementUseCount(i::Address top) {
+ if (last_top_ != top) {
+ use_count_ = 0;
+ last_top_ = top;
+ }
+ ++use_count_;
+ }
+
+ // How close to the new space top a fresh string has to be.
+ static const int kFreshnessLimit = 1024;
+
+ // The number of uses required to consider a string useful.
+ static const int kUseLimit = 32;
+
+ // Single use counter shared by all fresh strings.
+ static int use_count_;
+
+ // Last new space top when the use count above was valid.
+ static i::Address last_top_;
+};
+
+int StringTracker::use_count_ = 0;
+i::Address StringTracker::last_top_ = NULL;
+
+} // namespace
+
+
int String::Length() const {
if (IsDeadCheck("v8::String::Length()")) return 0;
return Utils::OpenHandle(this)->length();
@@ -2475,6 +2538,7 @@ int String::WriteUtf8(char* buffer, int capacity) const {
LOG_API("String::WriteUtf8");
ENTER_V8;
i::Handle<i::String> str = Utils::OpenHandle(this);
+ StringTracker::RecordWrite(str);
write_input_buffer.Reset(0, *str);
int len = str->length();
// Encode the first K - 3 bytes directly into the buffer since we
@@ -2518,6 +2582,7 @@ int String::WriteAscii(char* buffer, int start, int length) const {
ENTER_V8;
ASSERT(start >= 0 && length >= -1);
i::Handle<i::String> str = Utils::OpenHandle(this);
+ StringTracker::RecordWrite(str);
// Flatten the string for efficiency. This applies whether we are
// using StringInputBuffer or Get(i) to access the characters.
str->TryFlattenIfNotFlat();
@@ -2544,6 +2609,7 @@ int String::Write(uint16_t* buffer, int start, int length) const {
ENTER_V8;
ASSERT(start >= 0 && length >= -1);
i::Handle<i::String> str = Utils::OpenHandle(this);
+ StringTracker::RecordWrite(str);
int end = length;
if ( (length == -1) || (length > str->length() - start) )
end = str->length() - start;
@@ -3111,6 +3177,7 @@ bool v8::String::MakeExternal(v8::String::ExternalStringResource* resource) {
if (this->IsExternal()) return false; // Already an external string.
ENTER_V8;
i::Handle<i::String> obj = Utils::OpenHandle(this);
+ if (StringTracker::IsFreshUnusedString(obj)) return false;
bool result = obj->MakeExternal(resource);
if (result && !obj->IsSymbol()) {
i::ExternalStringTable::AddString(*obj);
@@ -3136,6 +3203,7 @@ bool v8::String::MakeExternal(
if (this->IsExternal()) return false; // Already an external string.
ENTER_V8;
i::Handle<i::String> obj = Utils::OpenHandle(this);
+ if (StringTracker::IsFreshUnusedString(obj)) return false;
bool result = obj->MakeExternal(resource);
if (result && !obj->IsSymbol()) {
i::ExternalStringTable::AddString(*obj);
@@ -3147,6 +3215,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 (StringTracker::IsFreshUnusedString(obj)) return false;
int size = obj->Size(); // Byte size of the original string.
if (size < i::ExternalString::kSize)
return false;
« no previous file with comments | « no previous file | test/cctest/test-api.cc » ('j') | test/cctest/test-api.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698