[Webkit-unassigned] [Bug 30209] Cache::m_liveDecodedResource is not sorted by CachedResource::m_lastDecodedAccessTime

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=30209





--- Comment #5 from Geoffrey Garen <ggaren at apple.com>  2009-10-19 09:21:25 PDT ---
Adam:

I disagree with r+ing this patch.

Holger said he experienced serious performance problems with earlier versions,
and it's not clear that those problems have been resolved. He also said that
this patch "has a problem".

Please consider my comments to Holger -- I think an r- might be appropriate
here, at least until Holger has a chance to respond.

Holger:

Storing a painting time in FrameView was an effort to avoid excessive calls to
currentTime(), which can be expensive. It would be nice if you could piggy-back
on that stored value, but unfortunately it gets set to 0 when painting ends.
Maybe we could invent something similar that you could piggy-back on.

I'm worried that these new calls to currentTime() will be a performance
problem. Did you test performance with this latest patch? If so, what were your
results?

Also, what user-visible bug are you fixing? I'm having a hard time
understanding what problem arises as a result of a CachedImage being inserted
into the LRU list with an initial m_lastDecodedAccessTime of 0. I see that
m_liveDecodedResources is assumed to be ordered from most recently to least
recently accessed, but I don't see a place where tying that order to
m_lastDecodedAccessTime is important. The one thing I do see is this:

            double elapsedTime = currentTime -
current->m_lastDecodedAccessTime;
            if (elapsedTime < cMinDelayBeforeLiveDecodedPrune)
                return;

Technically, an m_lastDecodedAccessTime of 0 could cause thrash in this case,
by allowing the cache to destroy decoded data on a resource that had just
decoded. However, that seems almost impossible in practice, since that one
resource would need to be big enough to bust the Cache size limits all by
itself. Have you seen something like that happen?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.