55091 2011-02-23 15:11:55 -0800 VectorBuffer should not call malloc(0) 2011-02-25 18:32:10 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Other OS X 10.5 RESOLVED FIXED P2 Normal --- 48719 1 eric eric abarth aestes commit-queue eric jamesr mjs ossy webkit.review.bot oldest_to_newest 356468 0 eric 2011-02-23 15:11:55 -0800 VectorBuffer should not call malloc(0) 356470 1 83558 eric 2011-02-23 15:14:25 -0800 Created attachment 83558 Patch 356472 2 eric 2011-02-23 15:14:47 -0800 This should be a simple, non-controversial review. :) 356474 3 eric 2011-02-23 15:15:37 -0800 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. 356482 4 83558 abarth 2011-02-23 15:22:57 -0800 Comment on attachment 83558 Patch Does vector buffer understand null pointers? Do we need to initialize anything else to null? 356484 5 eric 2011-02-23 15:24:09 -0800 (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. 356675 6 ap 2011-02-23 22:13:52 -0800 We usually ask for performance testing results for optimization patches. 357155 7 83558 commit-queue 2011-02-24 10:44:41 -0800 Comment on attachment 83558 Patch Clearing flags on attachment: 83558 Committed r79590: <http://trac.webkit.org/changeset/79590> 357156 8 commit-queue 2011-02-24 10:44:48 -0800 All reviewed patches have been landed. Closing bug. 357507 9 webkit.review.bot 2011-02-24 15:59:07 -0800 http://trac.webkit.org/changeset/79590 might have broken GTK Linux 32-bit Release 358332 10 jamesr 2011-02-25 18:19:00 -0800 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. 358342 11 eric 2011-02-25 18:32:10 -0800 I would expect this to be a general perf win on most benchmarks, yes. Suddenly copying SegmentedStrings got a lot cheaper. :) 83558 2011-02-23 15:14:25 -0800 2011-02-24 10:44:41 -0800 Patch bug-55091-20110223151423.patch text/plain 2341 eric U3VidmVyc2lvbiBSZXZpc2lvbjogNzk0NzkKZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0 Q29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCmluZGV4IDg2 OTYwZTZlZTQ0MjA0YmJlOTgxZmY4MThiZjYyZTRjZTI3YWNhOWUuLjg2MDM4YjZkNWQyNDQ0MTgw ODc1MDAwMTA3Mzk0NDk1MTMzN2Y4NjYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwpAQCAtMSwz ICsxLDIzIEBACisyMDExLTAyLTIzICBFcmljIFNlaWRlbCAgPGVyaWNAd2Via2l0Lm9yZz4KKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBWZWN0b3JCdWZm ZXIgc2hvdWxkIG5vdCBjYWxsIG1hbGxvYygwKQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0 Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NTUwOTEKKworICAgICAgICBUdXJucyBvdXQgdGhlIG1hbGxv YygpIGNhbGwgd2hpY2ggd2FzIHNvIGhvdCBpbjoKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTU1MDA1CisgICAgICAgIHdhcyBhY3R1YWxseSBqdXN0IG1h bGxvYygwKS4KKworICAgICAgICBXZSBzaG91bGRuJ3QgYmUgY2FsbGluZyBtYWxsb2MoMCkgYW55 d2F5LCBzaW5jZSB0aGVyZSBpcyBubyBuZWVkIHRvCisgICAgICAgIGFuZCBpdCBtaWdodCBhY3R1 YWxseSBkbyB3b3JrIG9uIHNvbWUgc3lzdGVtcy4KKyAgICAgICAgSSBiZWxpZXZlIG9uIE1hYyBp dCBlbmRzIHVwIHRha2luZyB0aGUgc3RhbmRhcmQgc3BpbmxvY2tzIChleHBlbnNpdmUpCisgICAg ICAgIGFuZCB0aGUgY29kZSBvbiBCcmV3IGFjdHVhbGx5IGRvZXMgYSBtYWxsb2MoMSkgaW5zdGVh ZC4gIE5laXRoZXIgaXMgZGVzaXJhYmxlLgorCisgICAgICAgICogd3RmL1ZlY3Rvci5oOgorICAg ICAgICAoV1RGOjpWZWN0b3JCdWZmZXJCYXNlOjphbGxvY2F0ZUJ1ZmZlcik6CisgICAgICAgIChX VEY6OlZlY3RvckJ1ZmZlckJhc2U6OnRyeUFsbG9jYXRlQnVmZmVyKToKKwogMjAxMS0wMi0yMyAg QWRhbSBSb2JlbiAgPGFyb2JlbkBhcHBsZS5jb20+CiAKICAgICAgICAgRml4IGFuIG9mZi1ieS1v bmUgZXJyb3IgaW4gSlNDOjphcHBlbmRTb3VyY2VUb0Vycm9yCmRpZmYgLS1naXQgYS9Tb3VyY2Uv SmF2YVNjcmlwdENvcmUvd3RmL1ZlY3Rvci5oIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL3d0Zi9W ZWN0b3IuaAppbmRleCA2ZDhkZDRjM2EzNTE3YjA1ZWE1MjA0NTJhYjBkMTY3MWM4Y2RkYTQzLi5j NzY1NGRmZGQ0NjM3ZDZiYjliYzA3NzEzZWE5ZTQxM2NlNGFhYjBiIDEwMDY0NAotLS0gYS9Tb3Vy Y2UvSmF2YVNjcmlwdENvcmUvd3RmL1ZlY3Rvci5oCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29y ZS93dGYvVmVjdG9yLmgKQEAgLTI4Miw2ICsyODIsNyBAQCBuYW1lc3BhY2UgV1RGIHsKICAgICBw dWJsaWM6CiAgICAgICAgIHZvaWQgYWxsb2NhdGVCdWZmZXIoc2l6ZV90IG5ld0NhcGFjaXR5KQog ICAgICAgICB7CisgICAgICAgICAgICBBU1NFUlQobmV3Q2FwYWNpdHkpOwogICAgICAgICAgICAg bV9jYXBhY2l0eSA9IG5ld0NhcGFjaXR5OwogICAgICAgICAgICAgaWYgKG5ld0NhcGFjaXR5ID4g c3RkOjpudW1lcmljX2xpbWl0czxzaXplX3Q+OjptYXgoKSAvIHNpemVvZihUKSkKICAgICAgICAg ICAgICAgICBDUkFTSCgpOwpAQCAtMjkwLDYgKzI5MSw3IEBAIG5hbWVzcGFjZSBXVEYgewogCiAg ICAgICAgIGJvb2wgdHJ5QWxsb2NhdGVCdWZmZXIoc2l6ZV90IG5ld0NhcGFjaXR5KQogICAgICAg ICB7CisgICAgICAgICAgICBBU1NFUlQobmV3Q2FwYWNpdHkpOwogICAgICAgICAgICAgaWYgKG5l d0NhcGFjaXR5ID4gc3RkOjpudW1lcmljX2xpbWl0czxzaXplX3Q+OjptYXgoKSAvIHNpemVvZihU KSkKICAgICAgICAgICAgICAgICByZXR1cm4gZmFsc2U7CiAKQEAgLTM2MCw3ICszNjIsMTAgQEAg bmFtZXNwYWNlIFdURiB7CiAKICAgICAgICAgVmVjdG9yQnVmZmVyKHNpemVfdCBjYXBhY2l0eSkK ICAgICAgICAgewotICAgICAgICAgICAgYWxsb2NhdGVCdWZmZXIoY2FwYWNpdHkpOworICAgICAg ICAgICAgLy8gQ2FsbGluZyBtYWxsb2MoMCkgbWlnaHQgdGFrZSBhIGxvY2sgYW5kIG1heSBhY3R1 YWxseSBkbyBhbgorICAgICAgICAgICAgLy8gYWxsb2NhdGlvbiBvbiBzb21lIHN5c3RlbXMgKGUu Zy4gQnJldykuCisgICAgICAgICAgICBpZiAoY2FwYWNpdHkpCisgICAgICAgICAgICAgICAgYWxs b2NhdGVCdWZmZXIoY2FwYWNpdHkpOwogICAgICAgICB9CiAKICAgICAgICAgflZlY3RvckJ1ZmZl cigpCg==