Revision: 6353
Author: [email protected]
Date: Mon Jan 17 08:54:56 2011
Log: Do not use possibly stale values for cache size, etc.
Those value can become invalid if cache gets cleared by GC.
Review URL: http://codereview.chromium.org/6348002
http://code.google.com/p/v8/source/detail?r=6353
Modified:
/branches/bleeding_edge/src/objects-debug.cc
/branches/bleeding_edge/src/objects-inl.h
/branches/bleeding_edge/src/objects.h
/branches/bleeding_edge/src/runtime.cc
=======================================
--- /branches/bleeding_edge/src/objects-debug.cc Sun Jan 16 23:03:19 2011
+++ /branches/bleeding_edge/src/objects-debug.cc Mon Jan 17 08:54:56 2011
@@ -670,16 +670,17 @@
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();
- get(i + 1)->Verify();
- // Key and value must be either both the holes, or not.
- ASSERT(get(i)->IsTheHole() == get(i + 1)->IsTheHole());
+ }
+ for (int i = size; i < length(); i++) {
+ ASSERT(get(i)->IsTheHole());
+ get(i)->Verify();
}
}
}
=======================================
--- /branches/bleeding_edge/src/objects-inl.h Mon Jan 17 00:11:03 2011
+++ /branches/bleeding_edge/src/objects-inl.h Mon Jan 17 08:54:56 2011
@@ -1978,19 +1978,39 @@
void JSFunctionResultCache::MakeZeroSize() {
- set(kFingerIndex, Smi::FromInt(kEntriesIndex));
- set(kCacheSizeIndex, Smi::FromInt(kEntriesIndex));
+ set_finger_index(kEntriesIndex);
+ set_size(kEntriesIndex);
}
void JSFunctionResultCache::Clear() {
- int cache_size = Smi::cast(get(kCacheSizeIndex))->value();
+ int cache_size = size();
Object** entries_start = RawField(this,
OffsetOfElementAt(kEntriesIndex));
MemsetPointer(entries_start,
Heap::the_hole_value(),
cache_size - kEntriesIndex);
MakeZeroSize();
}
+
+
+int JSFunctionResultCache::size() {
+ return Smi::cast(get(kCacheSizeIndex))->value();
+}
+
+
+void JSFunctionResultCache::set_size(int size) {
+ set(kCacheSizeIndex, Smi::FromInt(size));
+}
+
+
+int JSFunctionResultCache::finger_index() {
+ return Smi::cast(get(kFingerIndex))->value();
+}
+
+
+void JSFunctionResultCache::set_finger_index(int finger_index) {
+ set(kFingerIndex, Smi::FromInt(finger_index));
+}
byte ByteArray::get(int index) {
=======================================
--- /branches/bleeding_edge/src/objects.h Wed Jan 12 06:14:14 2011
+++ /branches/bleeding_edge/src/objects.h Mon Jan 17 08:54:56 2011
@@ -2613,6 +2613,11 @@
inline void MakeZeroSize();
inline void Clear();
+ inline int size();
+ inline void set_size(int size);
+ inline int finger_index();
+ inline void set_finger_index(int finger_index);
+
// Casting
static inline JSFunctionResultCache* cast(Object* obj);
=======================================
--- /branches/bleeding_edge/src/runtime.cc Mon Jan 17 00:11:03 2011
+++ /branches/bleeding_edge/src/runtime.cc Mon Jan 17 08:54:56 2011
@@ -10652,53 +10652,14 @@
UNREACHABLE();
return NULL;
}
-
-
-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);
- Handle<Object> key(key_obj);
- Handle<JSFunction> factory(JSFunction::cast(
- cache->get(JSFunctionResultCache::kFactoryIndex)));
- // TODO(antonm): consider passing a receiver when constructing a cache.
- Handle<Object> receiver(Top::global_context()->global());
-
- Handle<Object> value;
- {
- // This handle is nor shared, nor used later, so it's safe.
- Object** argv[] = { key.location() };
- bool pending_exception = false;
- value = Execution::Call(factory,
- receiver,
- 1,
- argv,
- &pending_exception);
- if (pending_exception) return Failure::Exception();
- }
-
- cache->set(index, *key);
- cache->set(index + 1, *value);
- cache->set(JSFunctionResultCache::kFingerIndex, Smi::FromInt(index));
-
- return *value;
-}
static MaybeObject* Runtime_GetFromCache(Arguments args) {
// This is only called from codegen, so checks might be more lax.
- CONVERT_CHECKED(FixedArray, cache, args[0]);
+ CONVERT_CHECKED(JSFunctionResultCache, cache, args[0]);
Object* key = args[1];
- const int finger_index =
- Smi::cast(cache->get(JSFunctionResultCache::kFingerIndex))->value();
-
+ int finger_index = cache->finger_index();
Object* o = cache->get(finger_index);
if (o == key) {
// The fastest case: hit the same place again.
@@ -10710,35 +10671,78 @@
i -= 2) {
o = cache->get(i);
if (o == key) {
- cache->set(JSFunctionResultCache::kFingerIndex, Smi::FromInt(i));
+ cache->set_finger_index(i);
return cache->get(i + 1);
}
}
- const int size =
-
Smi::cast(cache->get(JSFunctionResultCache::kCacheSizeIndex))->value();
+ int size = cache->size();
ASSERT(size <= cache->length());
for (int i = size - 2; i > finger_index; i -= 2) {
o = cache->get(i);
if (o == key) {
- cache->set(JSFunctionResultCache::kFingerIndex, Smi::FromInt(i));
+ cache->set_finger_index(i);
return cache->get(i + 1);
}
}
- // 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);
+ // There is no value in the cache. Invoke the function and cache result.
+ HandleScope scope;
+
+ Handle<JSFunctionResultCache> cache_handle(cache);
+ Handle<Object> key_handle(key);
+ Handle<Object> value;
+ {
+ Handle<JSFunction> factory(JSFunction::cast(
+ cache_handle->get(JSFunctionResultCache::kFactoryIndex)));
+ // TODO(antonm): consider passing a receiver when constructing a cache.
+ Handle<Object> receiver(Top::global_context()->global());
+ // This handle is nor shared, nor used later, so it's safe.
+ Object** argv[] = { key_handle.location() };
+ bool pending_exception = false;
+ value = Execution::Call(factory,
+ receiver,
+ 1,
+ argv,
+ &pending_exception);
+ if (pending_exception) return Failure::Exception();
+ }
+
+#ifdef DEBUG
+ cache_handle->JSFunctionResultCacheVerify();
+#endif
+
+ // Function invocation may have cleared the cache. Reread all the data.
+ finger_index = cache_handle->finger_index();
+ size = cache_handle->size();
+
+ // If we have spare room, put new data into it, otherwise evict post
finger
+ // entry which is likely to be the least recently used.
+ int index = -1;
+ if (size < cache_handle->length()) {
+ cache_handle->set_size(size + JSFunctionResultCache::kEntrySize);
+ index = size;
} else {
- int target_index = finger_index + JSFunctionResultCache::kEntrySize;
- if (target_index == cache->length()) {
- target_index = JSFunctionResultCache::kEntriesIndex;
- }
- return CacheMiss(cache, target_index, key);
- }
+ index = finger_index + JSFunctionResultCache::kEntrySize;
+ if (index == cache_handle->length()) {
+ index = JSFunctionResultCache::kEntriesIndex;
+ }
+ }
+
+ ASSERT(index % 2 == 0);
+ ASSERT(index >= JSFunctionResultCache::kEntriesIndex);
+ ASSERT(index < cache_handle->length());
+
+ cache_handle->set(index, *key_handle);
+ cache_handle->set(index + 1, *value);
+ cache_handle->set_finger_index(index);
+
+#ifdef DEBUG
+ cache_handle->JSFunctionResultCacheVerify();
+#endif
+
+ return *value;
}
#ifdef DEBUG
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
