Changeset 50213 in webkit

Timestamp:

Oct 28, 2009 4:44:23 AM (15 years ago)

Author:

zecke@webkit.org

Message:

Document a feature of the m_liveDecodedResources list.

​https://bugs.webkit.org/show_bug.cgi?id=30209
Reviewed by Darin Adler.

Document a feature of the m_liveDecodedResources list.
​https://bugs.webkit.org/show_bug.cgi?id=30209

The code made the assumption that the list is sorted by
the m_lastDecodedAccessTime property of the CachedResource.
The above is not true when CachedResource::setDecodedSize
is called and the item is inserted the first time. In this
case the m_lastDecodedAccessTime is still zero and the
m_liveDecodedResources list becomes unsorted.

It is impossible that Cache::pruneLiveResources will
stop to process the list too early due this feature and
the alternatives of updating m_lastDecodedAccessTime in
CachedResource::setDecodedSize or changing the insert
to search the right position have a negative impact on
performance. The best solution for now is to document
this feature.

• loader/Cache.cpp:

(WebCore::Cache::pruneLiveResources):

• loader/CachedResource.cpp:

(WebCore::CachedResource::setDecodedSize):

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• loader/Cache.cpp (modified) (1 diff)
• loader/CachedResource.cpp (modified) (1 diff)

trunk/WebCore/ChangeLog

@@ +1 @@
+2009-10-26  Holger Hans Peter Freyther  <zecke@selfish.org>
+
+        Reviewed by Darin Adler.
+
+        Document a feature of the m_liveDecodedResources list.
+        https://bugs.webkit.org/show_bug.cgi?id=30209
+
+        The code made the assumption that the list is sorted by
+        the m_lastDecodedAccessTime property of the CachedResource.
+        The above is not true when CachedResource::setDecodedSize
+        is called and the item is inserted the first time. In this
+        case the m_lastDecodedAccessTime is still zero and the
+        m_liveDecodedResources list becomes unsorted.
+
+        It is impossible that Cache::pruneLiveResources will
+        stop to process the list too early due this feature and
+        the alternatives of updating m_lastDecodedAccessTime in
+        CachedResource::setDecodedSize or changing the insert
+        to search the right position have a negative impact on
+        performance. The best solution for now is to document
+        this feature.
+
+        * loader/Cache.cpp:
+        (WebCore::Cache::pruneLiveResources):
+        * loader/CachedResource.cpp:
+        (WebCore::CachedResource::setDecodedSize):
+
 2009-10-28  Xan Lopez  <xlopez@igalia.com>
 

trunk/WebCore/loader/Cache.cpp

@@ -276 +276 @@
     // Destroy any decoded data in live objects that we can.
     // Start from the tail, since this is the least recently accessed of the objects.
+
+    // The list might not be sorted by the m_lastDecodedAccessTime. The impact
+    // of this weaker invariant is minor as the below if statement to check the
+    // elapsedTime will evaluate to false as the currentTime will be a lot
+    // greater than the current->m_lastDecodedAccessTime.
+    // For more details see: https://bugs.webkit.org/show_bug.cgi?id=30209
     CachedResource* current = m_liveDecodedResources.m_tail;
     while (current) {

trunk/WebCore/loader/CachedResource.cpp

@@ -243 +243 @@
         
         // Insert into or remove from the live decoded list if necessary.
+        // When inserting into the LiveDecodedResourcesList it is possible
+        // that the m_lastDecodedAccessTime is still zero or smaller than
+        // the m_lastDecodedAccessTime of the current list head. This is a
+        // violation of the invariant that the list is to be kept sorted
+        // by access time. The weakening of the invariant does not pose
+        // a problem. For more details please see: https://bugs.webkit.org/show_bug.cgi?id=30209
         if (m_decodedSize && !m_inLiveDecodedResourcesList && hasClients())
             cache()->insertInLiveDecodedResourcesList(this);