[Webkit-unassigned] [Bug 23732] Rework CachedResource::overheadSize accounting

[bugzilla-daemon at webkit.org]

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


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #27318|review?                     |review-
               Flag|                            |




------- Comment #2 from darin at apple.com  2009-02-04 10:28 PDT -------
(From update of attachment 27318)
> -    // FIXME: Find some programmatic lighweight way to calculate response size, and size of the different CachedResource classes.  
> -    // This is a rough estimate of resource overhead based on stats collected from the stress test.
> -    return sizeof(CachedResource) + 3648;
> -    
> -    /*  sizeof(CachedResource) + 
> -        192 +                        // average size of m_url.
> -        384 +                        // average size of m_clients hash map.
> -        1280 * 2 +                   // average size of ResourceResponse.  Doubled to account for the WebCore copy and the CF copy. 
> -                                     // Mostly due to the size of the hash maps, the Header Map strings and the URL.
> -        256 * 2                      // Overhead from ResourceRequest, doubled to account for WebCore copy and CF copy. 
> -                                     // Mostly due to the URL and Header Map.
> -    */
> +    return sizeof(CachedResource) +
> +        m_response.size() +
> +        m_url.length() * 2 +
> +        384; // average size of m_clients hash map

If we're not going to have a comment on each line, then I think we should write
the expression on one line in a more normal fashion.

I don't think size() is a good name for the "amount of memory used" function in
ResourceResponse. We use that name for simpler things such as the size of a
vector, and I'd prefer to use a name that makes more clear that this is
supposed to be a deep size that includes malloc overhead and other things like
that.

I don't think that the URL length * 2 is a great way to account for the size of
the URL, and using it is a change in behavior from the old version. It's true
that the characters are 2 bytes each (that should be sizeof(UChar) for clarity,
not 2), but just computing the length ignores malloc overhead and other
overhead of the various memory blocks used String (or is it KURL?) class.

> +    unsigned size() const { return 0; }

Is 0 a good default? Wouldn't some arbitrary non-zero number be better for
ports that don't go out of their way to optimize this?

I'm going to say review- because of the URL length issue above.


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