Bug 55091

Summary: VectorBuffer should not call malloc(0)
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: New BugsAssignee: Eric Seidel (no email) <eric>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, aestes, commit-queue, eric, jamesr, mjs, ossy, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 48719    
Attachments:
Description Flags
Patch none

Eric Seidel (no email)
Reported 2011-02-23 15:11:55 PST
VectorBuffer should not call malloc(0)
Attachments
Patch (2.29 KB, patch)
2011-02-23 15:14 PST, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1 2011-02-23 15:14:25 PST
Created attachment 83558 [details] Patch
Eric Seidel (no email)
Comment 2 2011-02-23 15:14:47 PST
This should be a simple, non-controversial review. :)
Eric Seidel (no email)
Comment 3 2011-02-23 15:15:37 PST
It does not seem to make tiny-innerHTML faster on my machine (but I no longer believe I can tell anything from that benchmark given how fast it runs), but it does make one more malloc disappear from the sample, which is a win.
Adam Barth
Comment 4 2011-02-23 15:22:57 PST
Comment on attachment 83558 [details] Patch Does vector buffer understand null pointers? Do we need to initialize anything else to null?
Eric Seidel (no email)
Comment 5 2011-02-23 15:24:09 PST
(In reply to comment #4) > (From update of attachment 83558 [details]) > Does vector buffer understand null pointers? Do we need to initialize anything else to null? VectorBufferBase does all the initialization for us. If you look at the 10 lines previous to this diff you'll see that doing nothing here is exactly what the default constructor does.
Alexey Proskuryakov
Comment 6 2011-02-23 22:13:52 PST
We usually ask for performance testing results for optimization patches.
WebKit Commit Bot
Comment 7 2011-02-24 10:44:41 PST
Comment on attachment 83558 [details] Patch Clearing flags on attachment: 83558 Committed r79590: <http://trac.webkit.org/changeset/79590>
WebKit Commit Bot
Comment 8 2011-02-24 10:44:48 PST
All reviewed patches have been landed. Closing bug.
WebKit Review Bot
Comment 9 2011-02-24 15:59:07 PST
http://trac.webkit.org/changeset/79590 might have broken GTK Linux 32-bit Release
James Robinson
Comment 10 2011-02-25 18:19:00 PST
We saw a nice perf win on several page cyclers when this changed rolled into Chromium: http://build.chromium.org/f/chromium/perf/mac-release-10.5/intl2/report.html?history=50&rev=76130 http://build.chromium.org/f/chromium/perf/mac-release-10.5/intl1/report.html?history=50&rev=76130 http://build.chromium.org/f/chromium/perf/linux-release-hardy/bloat-http/report.html?history=50&rev=76085 Think this was it? This wasn't the only change in the roll so it could have been something else.
Eric Seidel (no email)
Comment 11 2011-02-25 18:32:10 PST
I would expect this to be a general perf win on most benchmarks, yes. Suddenly copying SegmentedStrings got a lot cheaper. :)
Note You need to log in before you can comment on or make changes to this bug.