Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-30 Thread Alexey Proskuryakov
24 нояб. 2014 г., в 1:28, Antti Koivisto koivi...@iki.fi написал(а): I don't think this is really 32bit vs 64bit platform issue. The vast majority of 64bit systems our users have (that is iOS devices) can't use memory buffers sized anywhere near the 32bit limit even in theory. Also when

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-24 Thread Antti Koivisto
I don't think this is really 32bit vs 64bit platform issue. The vast majority of 64bit systems our users have (that is iOS devices) can't use memory buffers sized anywhere near the 32bit limit even in theory. Also when using Vector auto-grow capabilities (which is really the point of using a

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-20 Thread Carlos Garcia Campos
El mié, 19-11-2014 a las 12:20 -0800, Chris Dumez escribió: Hi all, I recently started updating the WTF::Vector API to use unsigned types instead of size_t [1][2], because: - WTF::Vector is already using unsigned type for size/capacity internally to save memory on 64-bit, causing a mismatch

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-20 Thread Alexey Proskuryakov
19 нояб. 2014 г., в 14:58, Alexey Proskuryakov a...@webkit.org написал(а): These and related uses are all over the place - see also Vectors in FormDataBuilder, data returned from FrameLoader::loadResourceSynchronously, plug-in code that loads from network, SharedBuffer etc. Another way to

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-20 Thread Alexey Proskuryakov
20 нояб. 2014 г., в 10:45, Filip Pizlo fpi...@apple.com написал(а): - uint64_t everywhere. This way, we'll solve practical problems with large resources once and for all. Also, this may prove to be necessary to solve even YouTube/Google Drive uploads, I do not know that yet. How does

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-20 Thread Geoffrey Garen
Yes, good point. I should have been more specific. I believe this math might overflow and then truncate: templatetypename T, unsigned inlineCapacity, typename OverflowHandler bool VectorT, inlineCapacity, OverflowHandler::tryExpandCapacity(unsigned newMinCapacity) { return

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-20 Thread Filip Pizlo
I think I'm starting to see what you're saying, so let me restate it my way to be sure: Code that deals with various loaded blobs must be resilient against them getting larger than a 32-bit size can handle and this is the subject of ongoing development effort. Such code may or may not use a

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-20 Thread Geoffrey Garen
I wonder what the downsides are to this approach. Footprint of Vector? It looks like the original change was motivated by shrinking Vector: https://bugs.webkit.org/show_bug.cgi?id=97268 https://bugs.webkit.org/show_bug.cgi?id=97268 Sadly, it didn’t include any data on the observed

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-20 Thread Filip Pizlo
If there was a benefit, then the following approach would make sense to me: - Have both Vector and SmallVector. SmallVector will use unsigned internally. - Use SmallVector in the places where sizeof(Vector) matters. This would take some effort but I think that we have the tools that would be

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-20 Thread Chris Dumez
The corresponding Blink bug did contain some performance data: CrBug#229226 https://code.google.com/p/chromium/issues/detail?id=229226 Kr, -- Chris Dumez - Apple Inc. Cupertino, CA On Nov 20, 2014, at 11:32 AM, Geoffrey Garen gga...@apple.com wrote: I wonder what the downsides are to this

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-20 Thread Filip Pizlo
That looks like a pretty good performance win. I'd advocate for SmallVector and Vector, then. -Filip On Nov 20, 2014, at 11:38 AM, Chris Dumez cdu...@apple.com wrote: The corresponding Blink bug did contain some performance data: CrBug#229226

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-20 Thread Antti Koivisto
Or Vector and ByteBuffer. All these cases requiring very large buffers seem to be about untyped data. antti On Thu, Nov 20, 2014 at 9:40 PM, Filip Pizlo fpi...@apple.com wrote: That looks like a pretty good performance win. I'd advocate for SmallVector and Vector, then. -Filip On

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-20 Thread Filip Pizlo
On Nov 20, 2014, at 2:06 PM, Alexey Proskuryakov a...@webkit.org wrote: SmallVector and Vector seem reasonable to me. I think that this is the right nomenclature - using limited size vectors should be a conscious choice. Even SmallVector should probably have a size_t API, so that we

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-20 Thread Maciej Stachowiak
On Nov 20, 2014, at 9:26 AM, Alexey Proskuryakov a...@webkit.org wrote: 19 нояб. 2014 г., в 14:58, Alexey Proskuryakov a...@webkit.org mailto:a...@webkit.org написал(а): These and related uses are all over the place - see also Vectors in FormDataBuilder, data returned from

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-20 Thread Maciej Stachowiak
On Nov 20, 2014, at 4:51 PM, Maciej Stachowiak m...@apple.com wrote: On Nov 20, 2014, at 9:26 AM, Alexey Proskuryakov a...@webkit.org mailto:a...@webkit.org wrote: 19 нояб. 2014 г., в 14:58, Alexey Proskuryakov a...@webkit.org mailto:a...@webkit.org написал(а): These and related

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-19 Thread Filip Pizlo
Whatever we do, the clients of Vector should be consistent about what type they use. I'm actually fine with Vector internally using unsigned even if the API uses size_t, but right now we have lots of code that uses a mix of size_t and unsigned when indexing into Vectors. That's confusing. If

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-19 Thread Alexey Proskuryakov
This is not exactly about Vector, but if one uses FileReader.prototype.readAsArrayBuffer() on a large file, I think that it overflows ArrayBuffer. WebKit actually crashes when uploading multi-gigabyte files to YouTube, Google Drive and other similar services, although I haven't checked

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-19 Thread Filip Pizlo
ArrayBuffers are very special because they are part of ECMAScript. We use unsigned for the length, because once upon a time, that would have been the right type; these days the right type would be a 53-bit integer. So, size_t would be wrong. I believe that ArrayBuffers should be changed to

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-19 Thread Antti Koivisto
I agree with Phil, we should just use unsigned consistently. For very large memory allocations Vector is probably not the right type to use. antti On Wed, Nov 19, 2014 at 10:32 PM, Filip Pizlo fpi...@apple.com wrote: Whatever we do, the clients of Vector should be consistent about what type

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-19 Thread Benjamin Poulain
I agree with Filip and Antti. The internal type of Vector is an orthogonal issue. Keeping the old size_t API with the unsigned internal has caused a mess of types in the codebase and some confusion for contributors. We should fix the API and clean up our code. There are cases were we can

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-19 Thread Alexey Proskuryakov
That makes good sense for internal implementation, do you think that class API should also use a custom type, or should it use size_t? With Vector though, I don't know how we would differentiate code paths that need large allocations from ones that don't. Nearly anything that is exposed as a

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-19 Thread Geoffrey Garen
I don’t haver a specific opinion on size_t vs unsigned, but I have a related point: If Vector uses unsigned, then it must RELEASE_ASSERT that it does not overflow unsigned when growing. Even if most normal content allocates 4GB, exploit content surely will try to allocate 4GB. Chris,

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-19 Thread Filip Pizlo
On Nov 19, 2014, at 1:50 PM, Alexey Proskuryakov a...@webkit.org wrote: That makes good sense for internal implementation, do you think that class API should also use a custom type, or should it use size_t? I would propose having an Int53 type in JSC, because that is now a thing in

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-19 Thread Chris Dumez
Hi, I believe the Vector class is already checking for overflows. I looked quickly and it seems that when trying to allocate / reallocate the Vector buffer, we will call CRASH() if: (newCapacity std::numeric_limitsunsigned::max() / sizeof(T)) Kr, -- Chris Dumez - Apple Inc. Cupertino, CA

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-19 Thread Oliver Hunt
On a related note I’ve also got a patch up for review that adds bounds checking to Vector::iterator, and that vigorously performs bounds and overflow checking. —Oliver On Nov 19, 2014, at 2:04 PM, Chris Dumez cdu...@apple.com wrote: Hi, I believe the Vector class is already checking for

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-19 Thread Alexey Proskuryakov
19 нояб. 2014 г., в 13:58, Filip Pizlo fpi...@apple.com написал(а): With Vector though, I don't know how we would differentiate code paths that need large allocations from ones that don't. Nearly anything that is exposed as a JS API or deals with external world can hit sizes over 4Gb.

Re: [webkit-dev] size_t vs unsigned in WTF::Vector API ?

2014-11-19 Thread Chris Dumez
If we don’t want to crash on overflow, the callers can use the try*() API I believe (e.g. tryAppend()). This returns false (and does not resize the vector) instead of crashing, when we reach the size limit. Kr, -- Chris Dumez - Apple Inc. Cupertino, CA On Nov 19, 2014, at 2:58 PM, Alexey