[Webkit-unassigned] [Bug 44806] Use purgeable memory to keep more dead resources in cache

[bugzilla-daemon at webkit.org]

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


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |darin at apple.com




--- Comment #7 from Darin Adler <darin at apple.com>  2010-08-30 15:46:31 PST ---
(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > Do we really need the optimization for a resource with a size of zero? Is that a common case that needs to be faster than other cases?
> 
> I'm leaving the check for 0 in to be consistent. If we want to get rid of the check, let me know and I can clean up all the call sites in a follow on bug.

Yes, I think we should get rid of the check.

> > > Is the static_cast here really needed? Why exactly?
> > This part of the code was copied from evict(). It does the static_cast as well. resource->size() returns an unsigned int so maybe the cast was added for that reason?
> 
> Leaving the static_cast in as well since resource->size() returns unsigned.

That’s not a good reason.

We can use a "-" with an unsigned. The extra cast doesn’t do anything helpful; just makes the code harder to read. It’s not good to copy that mistake from the other code.

> > > > -    if (isSafeToMakePurgeable())
> > > > -        makePurgeable(true);
> > > > +    if (!Cache::markMemoryPurgeableOnEvict()) {
> > > > +        if (isSafeToMakePurgeable())
> > > > +            makePurgeable(true);
> > > > +    }
> > > 
> > > I think && would be better here than nesting ifs, but also, can the check go lower level, inside isSafeToMakePurgeable and markPurgeable?
> > 
> > && is easy to do. I'll have to investigate if the check can go lower into makePurgeable(). 
> 
> It didn't look straightforward to move the check down to makePurgeable() so I'm leaving it where it is right now.

You don’t say anything concrete about why it didn’t look straightforward. It’s unfortunate to have checks in multiple places and wonder if we forgot one of the call sites.

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