[Webkit-unassigned] [Bug 115804] Avoid unnecessary data copies when loading subresources with DoNotBufferData option

[bugzilla-daemon at webkit.org]

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





--- Comment #34 from Carlos Garcia Campos <cgarcia at igalia.com>  2013-06-06 09:38:12 PST ---
(In reply to comment #33)
> (From update of attachment 203681 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=203681&action=review
> 
> >>> Source/WebCore/loader/cache/CachedImage.h:80
> >>> +    virtual void addData(const char* data, int length);
> >> 
> >> I don’t think int is the right type for length. The type unsigned could be OK, or maybe it needs to be size_t.
> > 
> > I think this comes from ResourceHandle::didReceiveData() that also uses int for length. I agree it should be size_t. ResourceHandle::didSendData() uses unsigned long long though.
> 
> Here’s my take on this:
> 
> 1) When we want to be able to hold the size of something, size_t is the type that can accommodate anything we could have in memory, so it’s always big enough.
> 
> 2) As an optimization to our data structures, many should use unsigned instead of size_t, since on 64-bit systems this can make our objects much smaller, and there are many things that simply can’t go past 2^32. I know that Andreas Kling and Antti are considering changes like this, including possibly changing Vector to use unsigned instead of size_t.
> 
> 3) When we are dealing with the size of a file on disk or something that can be dealt with without mapping or loading the whole thing into memory all at once, we want to use a type like "unsigned long long" that can represent sizes larger than 2^32 even on 32-bit systems. I think that’s why ResourceHandle::didSendData uses this type, because there are code paths where it can send data that does not need to be resident in memory all at the same time, possibly including very large files.

This piece would be useful to be somewhere in the wiki. Actually I think this comes from ResourceBuffer(const char*, int) that comes from SharedBuffer(const char* data, int size) that indeed has a FIXME:

// FIXME: Use unsigned consistently, and check for invalid casts when calling into SharedBuffer from other code.                                                                          
    if (size < 0)
        CRASH();

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