Reviewers: Vitaly,
Message:
Vitaly,
may you have a look?
Description:
Do not use possibly stale values for cache size, etc.
Those value can become invalid if cache gets cleared by GC.
Please review this at http://codereview.chromium.org/6348002/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/objects-debug.cc
M src/runtime.cc
Index: src/objects-debug.cc
diff --git a/src/objects-debug.cc b/src/objects-debug.cc
index
7d50bfb6f6ded8baee3b513d2f242314f4d10be3..f9c57e696078da0a7e2ab6e644f68ddf0f762861
100644
--- a/src/objects-debug.cc
+++ b/src/objects-debug.cc
@@ -670,16 +670,17 @@ void
JSFunctionResultCache::JSFunctionResultCacheVerify() {
int finger = Smi::cast(get(kFingerIndex))->value();
ASSERT(kEntriesIndex <= finger);
- ASSERT(finger < size || finger == kEntriesIndex);
+ ASSERT((finger < size) || (finger == kEntriesIndex && finger == size));
ASSERT_EQ(0, finger % kEntrySize);
if (FLAG_enable_slow_asserts) {
- STATIC_ASSERT(2 == kEntrySize);
- for (int i = kEntriesIndex; i < length(); i += kEntrySize) {
+ for (int i = kEntriesIndex; i < size; i++) {
+ ASSERT(!get(i)->IsTheHole());
+ get(i)->Verify();
+ }
+ for (int i = size; i < length(); i++) {
+ ASSERT(get(i)->IsTheHole());
get(i)->Verify();
- get(i + 1)->Verify();
- // Key and value must be either both the holes, or not.
- ASSERT(get(i)->IsTheHole() == get(i + 1)->IsTheHole());
}
}
}
Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index
0cde7779a3d3fbb817b9825129fd5442f0dc2af3..a0497086c73186dd7df9d9e60c8a9b08c24d3e95
100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -10655,12 +10655,7 @@ static MaybeObject* Runtime_Abort(Arguments args) {
MUST_USE_RESULT static MaybeObject* CacheMiss(FixedArray* cache_obj,
- int index,
Object* key_obj) {
- ASSERT(index % 2 == 0); // index of the key
- ASSERT(index >= JSFunctionResultCache::kEntriesIndex);
- ASSERT(index < cache_obj->length());
-
HandleScope scope;
Handle<FixedArray> cache(cache_obj);
@@ -10683,10 +10678,41 @@ MUST_USE_RESULT static MaybeObject*
CacheMiss(FixedArray* cache_obj,
if (pending_exception) return Failure::Exception();
}
+#ifdef DEBUG
+
Handle<JSFunctionResultCache>::cast(cache)->JSFunctionResultCacheVerify();
+#endif
+
+ // Function invocation can have cleared the cache. Reread all the data.
+ const int finger_index =
+ Smi::cast(cache->get(JSFunctionResultCache::kFingerIndex))->value();
+ const int size =
+
Smi::cast(cache->get(JSFunctionResultCache::kCacheSizeIndex))->value();
+
+ // If we have spare room, put new data into it, otherwise evict post
finger
+ // entry which is likely to be least recently used.
+ int index = -1;
+ if (size < cache->length()) {
+ cache->set(JSFunctionResultCache::kCacheSizeIndex, Smi::FromInt(size +
2));
+ index = size;
+ } else {
+ index = finger_index + JSFunctionResultCache::kEntrySize;
+ if (index == cache->length()) {
+ index = JSFunctionResultCache::kEntriesIndex;
+ }
+ }
+
+ ASSERT(index % 2 == 0);
+ ASSERT(index >= JSFunctionResultCache::kEntriesIndex);
+ ASSERT(index < cache_obj->length());
+
cache->set(index, *key);
cache->set(index + 1, *value);
cache->set(JSFunctionResultCache::kFingerIndex, Smi::FromInt(index));
+#ifdef DEBUG
+
Handle<JSFunctionResultCache>::cast(cache)->JSFunctionResultCacheVerify();
+#endif
+
return *value;
}
@@ -10727,18 +10753,7 @@ static MaybeObject* Runtime_GetFromCache(Arguments
args) {
}
}
- // Cache miss. If we have spare room, put new data into it, otherwise
- // evict post finger entry which must be least recently used.
- if (size < cache->length()) {
- cache->set(JSFunctionResultCache::kCacheSizeIndex, Smi::FromInt(size +
2));
- return CacheMiss(cache, size, key);
- } else {
- int target_index = finger_index + JSFunctionResultCache::kEntrySize;
- if (target_index == cache->length()) {
- target_index = JSFunctionResultCache::kEntriesIndex;
- }
- return CacheMiss(cache, target_index, key);
- }
+ return CacheMiss(cache, key);
}
#ifdef DEBUG
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
