Do not use possibly stale values for cache size, etc.

src/runtime.cc

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,

[Vitaly Repeshko, 2011/01/17 10:08:33]

The name is probably too generic for this >10K line file.

[antonm, 2011/01/17 16:46:06]

I've inlined this helper into the runtime function :)

-                                              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.

[Vitaly Repeshko, 2011/01/17 10:08:33]

nit: "may have" or "might have". "Can have" sounds more like could do, but
didn't.

[antonm, 2011/01/17 16:46:06]

Done.

+  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