37684 2010-04-15 16:58:12 -0700 [Qt] JSValue QtClass::fallbackObject can be optimized 2010-05-18 00:33:53 -0700 1 1 1 Unclassified WebKit WebKit Qt 528+ (Nightly build) PC All CLOSED FIXED Qt, QtTriaged P2 Enhancement --- 35784 1 agbakken agbakken commit-queue hausmann jesus kenneth kimmo.t.kinnunen oldest_to_newest 213111 0 53489 agbakken 2010-04-15 16:58:12 -0700 Created attachment 53489 patch that reduces memcpy'ing JSValue QtClass::fallbackObject(ExecState* exec, Instance* inst, const Identifier& identifier) copies and modifies more string data than it needs to. The attached patch improves this. 224172 1 jesus 2010-05-11 14:59:34 -0700 (In reply to comment #0) > The attached patch improves this. It is necessary to mention this bug report on the Changelog. Also, do you want it to be reviewed? If so you have to change it to r? . Kenneth, can you take a look at this? LGTM. ;) 224174 2 kenneth 2010-05-11 15:02:41 -0700 I guess you wanted to put this up for review 224493 3 agbakken 2010-05-12 02:58:07 -0700 I did want to put it up for review. Sorry about that :-) 224547 4 53489 kenneth 2010-05-12 05:39:26 -0700 Comment on attachment 53489 patch that reduces memcpy'ing Looks good Anders! 224563 5 53489 jesus 2010-05-12 06:15:27 -0700 Comment on attachment 53489 patch that reduces memcpy'ing I'll add the bug number to the changelog and upload a new patch. Just following the guidelines... ;) 224566 6 55837 jesus 2010-05-12 06:24:45 -0700 Created attachment 55837 Patch Added the bug number to the Changelog. 224567 7 53489 jesus 2010-05-12 06:25:19 -0700 Comment on attachment 53489 patch that reduces memcpy'ing Clearing flags of old patch. 225693 8 55837 commit-queue 2010-05-14 00:56:13 -0700 Comment on attachment 55837 Patch Rejecting patch 55837 from commit-queue. Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--force']" exit_code: 2 Parsed 2 diffs from patch file(s). patching file WebCore/ChangeLog patch: **** malformed patch at line 18: 2010-04-15 Adam Roben <aroben@apple.com> patching file WebCore/bridge/qt/qt_class.cpp Full output: http://webkit-commit-queue.appspot.com/results/2315058 225737 9 56062 agbakken 2010-05-14 03:19:44 -0700 Created attachment 56062 New version of the patch 226189 10 56062 commit-queue 2010-05-15 00:51:26 -0700 Comment on attachment 56062 New version of the patch Rejecting patch 56062 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--ignore-tests', 'compositing/iframes', '--quiet']" exit_code: 1 Last 500 characters of output: ng Java tests make: Nothing to be done for `default'. Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests Skipped list contained 'compositing/iframes/composited-iframe.html', but no file of that name could be found Testing 18341 test cases. fast/loader/recursive-before-unload-crash.html -> failed Exiting early after 1 failures. 13417 tests run. 198.65s total testing time 13416 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 4 test cases (<1%) had stderr output Full output: http://webkit-commit-queue.appspot.com/results/2265108 226303 11 kenneth 2010-05-15 12:32:49 -0700 Anders, you cannot remove the "Reviewed by NOBODY (OOPS!!)". If you do that we cannot land using the commit-queue. It has to be exactly like the prepare-ChangeLog scripts writes it. Now you can reupload the patch with this fixed and reset r? and cq? -- or -- You can upload the patch with "Reviewed by Kenneth Rohde Christiansen" and just set cq? The latter is preferred as anyone with committer-rights can change the cq? to cq+ Another option would be to get someone to land it manually using webkit-patch or similar. 226366 12 56062 abarth 2010-05-15 17:55:32 -0700 Comment on attachment 56062 New version of the patch I think you were just bit by a flaky test. 226393 13 56062 commit-queue 2010-05-15 19:52:32 -0700 Comment on attachment 56062 New version of the patch Clearing flags on attachment: 56062 Committed r59563: <http://trac.webkit.org/changeset/59563> 226394 14 commit-queue 2010-05-15 19:52:38 -0700 All reviewed patches have been landed. Closing bug. 226441 15 hausmann 2010-05-16 00:58:58 -0700 Revision r59563 cherry-picked into qtwebkit-2.0 with commit 4bb49bb59d4f763069ae6bb84b9648d6ed0bfd1d 226732 16 kimmo.t.kinnunen 2010-05-17 05:25:35 -0700 This commit causes a bug which prevents signals from being matched. It introduces second local variable named "index", which then overrides the proper method number variable (also called "index") this causes signals from being found in some cases. The affected line is: QtRuntimeMetaMethod* val = new (exec) QtRuntimeMetaMethod(exec, identifier, static_cast<QtInstance*>(inst), index, normal, false); 226733 17 kenneth 2010-05-17 05:32:04 -0700 Uh, I wasn't able to see that from the diff. I will fix it later today 226735 18 kenneth 2010-05-17 05:40:50 -0700 Patch shadowed a variable. 226743 19 kenneth 2010-05-17 06:50:28 -0700 Fixed regression in r59606 226760 20 agbakken 2010-05-17 07:37:32 -0700 Oops. My bad. 227166 21 hausmann 2010-05-18 00:29:30 -0700 <cherr-pick-for-backport: r59606> 227167 22 hausmann 2010-05-18 00:30:44 -0700 <cherry-pick-for-backport: r59606> 227168 23 hausmann 2010-05-18 00:33:41 -0700 Revision r59606 cherry-picked into qtwebkit-2.0 with commit 807157e42add842605ec67d9363dd3f1861748ca 53489 2010-04-15 16:58:12 -0700 2010-05-12 06:25:19 -0700 patch that reduces memcpy'ing patch text/plain 1465 agbakken SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1NzY4NSkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTIgQEAKKzIwMTAtMDQtMTUgIEFuZGVycyBCYWtrZW4gIDxhZ2Jha2tlbkBnbWFp bC5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg RG9uJ3QgdW5uZWNlc3NhcmlseSBjb3B5IGRhdGEgd2hlbiBzZWFyY2hpbmcgZm9yIG1ldGhvZHMg aW4gUXRDbGFzcy4KKworICAgICAgICAqIGJyaWRnZS9xdC9xdF9jbGFzcy5jcHA6CisgICAgICAg IChKU0M6OkJpbmRpbmdzOjpRdENsYXNzOjpmYWxsYmFja09iamVjdCk6CisKIDIwMTAtMDQtMTUg IEFkYW0gUm9iZW4gIDxhcm9iZW5AYXBwbGUuY29tPgogCiAgICAgICAgIEV4cG9zZSBVc2VyQ29u dGVudFVSTFBhdHRlcm4gYXMgV2ViS2l0IFNQSQpJbmRleDogV2ViQ29yZS9icmlkZ2UvcXQvcXRf Y2xhc3MuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvYnJpZGdlL3F0L3F0X2NsYXNzLmNwcAko cmV2aXNpb24gNTc2ODUpCisrKyBXZWJDb3JlL2JyaWRnZS9xdC9xdF9jbGFzcy5jcHAJKHdvcmtp bmcgY29weSkKQEAgLTk4LDEwICs5OCwxMiBAQCBKU1ZhbHVlIFF0Q2xhc3M6OmZhbGxiYWNrT2Jq ZWN0KEV4ZWNTdGF0CiAgICAgICAgIGlmIChtLmFjY2VzcygpID09IFFNZXRhTWV0aG9kOjpQcml2 YXRlKQogICAgICAgICAgICAgY29udGludWU7CiAKLSAgICAgICAgUUJ5dGVBcnJheSBzaWduYXR1 cmUgPSBtLnNpZ25hdHVyZSgpOwotICAgICAgICBzaWduYXR1cmUudHJ1bmNhdGUoc2lnbmF0dXJl LmluZGV4T2YoJygnKSk7CisgICAgICAgIGludCBpbmRleCA9IDA7CisgICAgICAgIGNvbnN0IGNo YXIqIHNpZ25hdHVyZSA9IG0uc2lnbmF0dXJlKCk7CisgICAgICAgIHdoaWxlIChzaWduYXR1cmVb aW5kZXhdICYmIHNpZ25hdHVyZVtpbmRleF0gIT0gJygnKQorICAgICAgICAgICAgKytpbmRleDsK IAotICAgICAgICBpZiAobm9ybWFsID09IHNpZ25hdHVyZSkgeworICAgICAgICBpZiAobm9ybWFs ID09IFFCeXRlQXJyYXk6OmZyb21SYXdEYXRhKHNpZ25hdHVyZSwgaW5kZXgpKSB7CiAgICAgICAg ICAgICBRdFJ1bnRpbWVNZXRhTWV0aG9kKiB2YWwgPSBuZXcgKGV4ZWMpIFF0UnVudGltZU1ldGFN ZXRob2QoZXhlYywgaWRlbnRpZmllciwgc3RhdGljX2Nhc3Q8UXRJbnN0YW5jZSo+KGluc3QpLCBp bmRleCwgbm9ybWFsLCBmYWxzZSk7CiAgICAgICAgICAgICBxdGluc3QtPm1fbWV0aG9kcy5pbnNl cnQobmFtZSwgdmFsKTsKICAgICAgICAgICAgIHJldHVybiB2YWw7Cg== 55837 2010-05-12 06:24:45 -0700 2010-05-14 00:56:12 -0700 Patch patch.patch text/plain 1603 jesus SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n ICAgKHJldmlzaW9uIDU3Njg1KQorKysgV2ViQ29yZS9DaGFuZ2VMb2cgICAod29ya2luZyBjb3B5 KQpAQCAtMSwzICsxLDEyIEBACisyMDEwLTA0LTE1ICBBbmRlcnMgQmFra2VuICA8YWdiYWtrZW5A Z21haWwuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IEtlbm5ldGggUm9oZGUgQ2hyaXN0aWFu c2VuLgorCisgICAgICAgIERvbid0IHVubmVjZXNzYXJpbHkgY29weSBkYXRhIHdoZW4gc2VhcmNo aW5nIGZvciBtZXRob2RzIGluIFF0Q2xhc3MuCisKKyAgICAgICAgW1F0XSBKU1ZhbHVlIFF0Q2xh c3M6OmZhbGxiYWNrT2JqZWN0IGNhbiBiZSBvcHRpbWl6ZWQKKyAgICAgICAgaHR0cHM6Ly9idWdz LndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTM3Njg0CisKKyAgICAgICAgKiBicmlkZ2UvcXQv cXRfY2xhc3MuY3BwOgorICAgICAgICAoSlNDOjpCaW5kaW5nczo6UXRDbGFzczo6ZmFsbGJhY2tP YmplY3QpOgorCiAyMDEwLTA0LTE1ICBBZGFtIFJvYmVuICA8YXJvYmVuQGFwcGxlLmNvbT4KIAog ICAgICAgICBFeHBvc2UgVXNlckNvbnRlbnRVUkxQYXR0ZXJuIGFzIFdlYktpdCBTUEkKSW5kZXg6 IFdlYkNvcmUvYnJpZGdlL3F0L3F0X2NsYXNzLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL2Jy aWRnZS9xdC9xdF9jbGFzcy5jcHAgIChyZXZpc2lvbiA1NzY4NSkKKysrIFdlYkNvcmUvYnJpZGdl L3F0L3F0X2NsYXNzLmNwcCAgKHdvcmtpbmcgY29weSkKQEAgLTk4LDEwICs5OCwxMiBAQCBKU1Zh bHVlIFF0Q2xhc3M6OmZhbGxiYWNrT2JqZWN0KEV4ZWNTdGF0CiAgICAgICAgIGlmIChtLmFjY2Vz cygpID09IFFNZXRhTWV0aG9kOjpQcml2YXRlKQogICAgICAgICAgICAgY29udGludWU7CiAKLSAg ICAgICAgUUJ5dGVBcnJheSBzaWduYXR1cmUgPSBtLnNpZ25hdHVyZSgpOwotICAgICAgICBzaWdu YXR1cmUudHJ1bmNhdGUoc2lnbmF0dXJlLmluZGV4T2YoJygnKSk7CisgICAgICAgIGludCBpbmRl eCA9IDA7CisgICAgICAgIGNvbnN0IGNoYXIqIHNpZ25hdHVyZSA9IG0uc2lnbmF0dXJlKCk7Cisg ICAgICAgIHdoaWxlIChzaWduYXR1cmVbaW5kZXhdICYmIHNpZ25hdHVyZVtpbmRleF0gIT0gJygn KQorICAgICAgICAgICAgKytpbmRleDsKIAotICAgICAgICBpZiAobm9ybWFsID09IHNpZ25hdHVy ZSkgeworICAgICAgICBpZiAobm9ybWFsID09IFFCeXRlQXJyYXk6OmZyb21SYXdEYXRhKHNpZ25h dHVyZSwgaW5kZXgpKSB7CiAgICAgICAgICAgICBRdFJ1bnRpbWVNZXRhTWV0aG9kKiB2YWwgPSBu ZXcgKGV4ZWMpIFF0UnVudGltZU1ldGFNZXRob2QoZXhlYywgaWRlbnRpZmllciwgc3RhdGljX2Nh c3Q8UXRJbnN0YW5jZSo+KGluc3QpLCBpbmRleCwgbm9ybWFsLCBmYWxzZSk7CiAgICAgICAgICAg ICBxdGluc3QtPm1fbWV0aG9kcy5pbnNlcnQobmFtZSwgdmFsKTsKICAgICAgICAgICAgIHJldHVy biB2YWw7Cg== 56062 2010-05-14 03:19:44 -0700 2010-05-15 19:52:32 -0700 New version of the patch patch.38949 text/plain 1585 agbakken SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1OTQ2MCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTUgQEAKKzIwMTAtMDQtMTUgIEFuZGVycyBCYWtrZW4gIDxhZ2Jha2tlbkBnbWFp bC5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgS2VubmV0aCBSb2hkZSBDaHJpc3RpYW5zZW4u CisKKyAgICAgICAgRG9uJ3QgdW5uZWNlc3NhcmlseSBjb3B5IGRhdGEgd2hlbiBzZWFyY2hpbmcg Zm9yIG1ldGhvZHMgaW4gUXRDbGFzcy4KKworICAgICAgICBbUXRdIEpTVmFsdWUgUXRDbGFzczo6 ZmFsbGJhY2tPYmplY3QgY2FuIGJlIG9wdGltaXplZAorICAgICAgICBodHRwczovL2J1Z3Mud2Vi a2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9Mzc2ODQKKworICAgICAgICAqIGJyaWRnZS9xdC9xdF9j bGFzcy5jcHA6CisgICAgICAgIChKU0M6OkJpbmRpbmdzOjpRdENsYXNzOjpmYWxsYmFja09iamVj dCk6CisKIDIwMTAtMDUtMTQgIEFhcm9uIEtlbm5lZHkgIDx0ZmZlZWJAZ21haWwuY29tPgogCiAg ICAgICAgIFJldmlld2VkIGJ5IFNpbW9uIEhhdXNtYW5uLgpJbmRleDogV2ViQ29yZS9icmlkZ2Uv cXQvcXRfY2xhc3MuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvYnJpZGdlL3F0L3F0X2NsYXNz LmNwcAkocmV2aXNpb24gNTk0NjApCisrKyBXZWJDb3JlL2JyaWRnZS9xdC9xdF9jbGFzcy5jcHAJ KHdvcmtpbmcgY29weSkKQEAgLTk4LDEwICs5OCwxMiBAQCBKU1ZhbHVlIFF0Q2xhc3M6OmZhbGxi YWNrT2JqZWN0KEV4ZWNTdGF0CiAgICAgICAgIGlmIChtLmFjY2VzcygpID09IFFNZXRhTWV0aG9k OjpQcml2YXRlKQogICAgICAgICAgICAgY29udGludWU7CiAKLSAgICAgICAgUUJ5dGVBcnJheSBz aWduYXR1cmUgPSBtLnNpZ25hdHVyZSgpOwotICAgICAgICBzaWduYXR1cmUudHJ1bmNhdGUoc2ln bmF0dXJlLmluZGV4T2YoJygnKSk7CisgICAgICAgIGludCBpbmRleCA9IDA7CisgICAgICAgIGNv bnN0IGNoYXIqIHNpZ25hdHVyZSA9IG0uc2lnbmF0dXJlKCk7CisgICAgICAgIHdoaWxlIChzaWdu YXR1cmVbaW5kZXhdICYmIHNpZ25hdHVyZVtpbmRleF0gIT0gJygnKQorICAgICAgICAgICAgKytp bmRleDsKIAotICAgICAgICBpZiAobm9ybWFsID09IHNpZ25hdHVyZSkgeworICAgICAgICBpZiAo bm9ybWFsID09IFFCeXRlQXJyYXk6OmZyb21SYXdEYXRhKHNpZ25hdHVyZSwgaW5kZXgpKSB7CiAg ICAgICAgICAgICBRdFJ1bnRpbWVNZXRhTWV0aG9kKiB2YWwgPSBuZXcgKGV4ZWMpIFF0UnVudGlt ZU1ldGFNZXRob2QoZXhlYywgaWRlbnRpZmllciwgc3RhdGljX2Nhc3Q8UXRJbnN0YW5jZSo+KGlu c3QpLCBpbmRleCwgbm9ybWFsLCBmYWxzZSk7CiAgICAgICAgICAgICBxdGluc3QtPm1fbWV0aG9k cy5pbnNlcnQobmFtZSwgdmFsKTsKICAgICAgICAgICAgIHJldHVybiB2YWw7Cg==