Attachment #41860 for bug #30209

View Details Raw Unified Return to Bug 30209

| Differences between



2009-10-26  Holger Hans Peter Freyther  <zecke@selfish.org>

        Reviewed by NOBODY (OOPS!).

        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-25  Anton Muhin  <antonm@chromium.org>

        Reviewed by Adam Barth.

Lines 275-280 void Cache::pruneLiveResources() a/WebCore/loader/Cache.cpp_sec1
- a/WebCore/loader/Cache.cpp +6 lines
275		275
276	// Destroy any decoded data in live objects that we can.	276	// Destroy any decoded data in live objects that we can.
277	// Start from the tail, since this is the least recently accessed of the objects.	277	// Start from the tail, since this is the least recently accessed of the objects.
		278
		279	// The list is not stricly sorted by the m_lastDecodedAccessTime but
		280	// the impact of this behavior is minor as the below if with the return
		281	// statement will not evaluate to true as the currentTime is >> than
		282	// current->m_lastDecodedAccessTime. For more details see:
		283	// https://bugs.webkit.org/show_bug.cgi?id=30209
278	CachedResource* current = m_liveDecodedResources.m_tail;	284	CachedResource* current = m_liveDecodedResources.m_tail;
279	while (current) {	285	while (current) {
280	CachedResource* prev = current->m_prevInLiveResourcesList;	286	CachedResource* prev = current->m_prevInLiveResourcesList;

Lines 241-247 void CachedResource::setDecodedSize(unsigned size) a/WebCore/loader/CachedResource.cpp_sec1
- a/WebCore/loader/CachedResource.cpp -2 / +5 lines
241	// Now insert into the new LRU list.	241	// Now insert into the new LRU list.
242	cache()->insertInLRUList(this);	242	cache()->insertInLRUList(this);
243		243
244	// Insert into or remove from the live decoded list if necessary.	244	// Insert into or remove from the live decoded list if necessary. When inserting
		245	// into the LiveDecodedResourcesList the m_lastDecodedAccessTime might still be
		246	// zero or smaller than the m_lastDecodedAccessTime of the current head of this
		247	// list. This is a violation of the current invariant but it is not a problem.
		248	// For more details please see: https://bugs.webkit.org/show_bug.cgi?id=30209
245	if (m_decodedSize && !m_inLiveDecodedResourcesList && hasClients())	249	if (m_decodedSize && !m_inLiveDecodedResourcesList && hasClients())
246	cache()->insertInLiveDecodedResourcesList(this);	250	cache()->insertInLiveDecodedResourcesList(this);
247	else if (!m_decodedSize && m_inLiveDecodedResourcesList)	251	else if (!m_decodedSize && m_inLiveDecodedResourcesList)
248	-

Return to Bug 30209

Lines 1-3 a/WebCore/ChangeLog_sec1
- a/WebCore/ChangeLog +27 lines
		1	2009-10-26 Holger Hans Peter Freyther <zecke@selfish.org>
		2
		3	Reviewed by NOBODY (OOPS!).
		4
		5	Document a feature of the m_liveDecodedResources list.
		6	https://bugs.webkit.org/show_bug.cgi?id=30209
		7
		8	The code made the assumption that the list is sorted by
		9	the m_lastDecodedAccessTime property of the CachedResource.
		10	The above is not true when CachedResource::setDecodedSize
		11	is called and the item is inserted the first time. In this
		12	case the m_lastDecodedAccessTime is still zero and the
		13	m_liveDecodedResources list becomes unsorted.
		14
		15	It is impossible that Cache::pruneLiveResources will
		16	stop to process the list too early due this feature and
		17	the alternatives of updating m_lastDecodedAccessTime in
		18	CachedResource::setDecodedSize or changing the insert
		19	to search the right position have a negative impact on
		20	performance. The best solution for now is to document
		21	this feature.
		22
		23	* loader/Cache.cpp:
		24	(WebCore::Cache::pruneLiveResources):
		25	* loader/CachedResource.cpp:
		26	(WebCore::CachedResource::setDecodedSize):
		27
1	2009-10-25 Anton Muhin <antonm@chromium.org>	28	2009-10-25 Anton Muhin <antonm@chromium.org>
2		29
3	Reviewed by Adam Barth.	30	Reviewed by Adam Barth.