102852 2012-11-20 16:50:22 -0800 [V8] 7% regression in dromaeo_domcorequery/dom_query_getElementsByTagName__not_in_document 2012-11-23 13:49:02 -0800 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 ojan abarth abarth eric haraken japhet webkit.review.bot oldest_to_newest 772294 0 ojan 2012-11-20 16:50:22 -0800 http://build.chromium.org/f/chromium/perf/chromium-rel-win7-webkit/dromaeo_domcorequery/report.html?rev=168911&graph=dom_query_getElementsByTagName__not_in_document_&history=50 Regression range looks to be: http://trac.webkit.org/log/?verbose=on&rev=135212&stop_rev=135193 Chromium side regression range is http://build.chromium.org/f/chromium/perf/dashboard/ui/changelog.html?url=/trunk/src&mode=html&range=168612:168679, but it seems unlikely to me that this is a chromium-side regression. http://trac.webkit.org/changeset/135208/ seems like the most likely culprit. 772301 1 abarth 2012-11-20 16:53:14 -0800 Rolling out my patch now. 772306 2 abarth 2012-11-20 16:55:11 -0800 The rollout has a conflict. :*( 772313 3 abarth 2012-11-20 17:07:15 -0800 I'm just going to try to fix this directly rather than rolling out. There have been a bunch of dependent patches landed already. 772319 4 175310 abarth 2012-11-20 17:18:10 -0800 Created attachment 175310 Patch 772324 5 175310 haraken 2012-11-20 17:24:49 -0800 Comment on attachment 175310 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=175310&action=review Looks reasonable. > Source/WebCore/bindings/v8/DOMDataStore.cpp:65 > + V8DOMWindowShell* shell = V8DOMWindowShell::isolated(v8::Context::GetEntered()); > + if (UNLIKELY(!!shell)) > + return shell->world()->isolatedWorldDOMDataStore(); Maybe you can simplify the code by using worldForEnteredContextIfIsolated()? 772326 6 175310 abarth 2012-11-20 17:27:39 -0800 Comment on attachment 175310 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=175310&action=review >> Source/WebCore/bindings/v8/DOMDataStore.cpp:65 >> + return shell->world()->isolatedWorldDOMDataStore(); > > Maybe you can simplify the code by using worldForEnteredContextIfIsolated()? That has an extra v8::Context::InContext() check that we don't need. 772327 7 haraken 2012-11-20 17:28:14 -0800 (In reply to comment #6) > > Maybe you can simplify the code by using worldForEnteredContextIfIsolated()? > > That has an extra v8::Context::InContext() check that we don't need. Makes sense. 772355 8 175310 webkit.review.bot 2012-11-20 18:16:11 -0800 Comment on attachment 175310 Patch Clearing flags on attachment: 175310 Committed r135339: <http://trac.webkit.org/changeset/135339> 772356 9 webkit.review.bot 2012-11-20 18:16:14 -0800 All reviewed patches have been landed. Closing bug. 772499 10 abarth 2012-11-20 23:14:56 -0800 That patch didn't seem to have healed it. 772720 11 abarth 2012-11-21 02:59:50 -0800 It actually made it slightly worse. I suspect bug 102854 is the path to salvation here, but I need to look at the profile. 773114 12 abarth 2012-11-21 10:49:58 -0800 As expected bug 102854 causes these functions to drop off the profile entirely. I just need to fix the worker issue. I'm realize we're not using the normal "revert and re-land approach"... We should either finish bug 102854 soon to verify that it heals the bot or we should figure out what would be involved in backing all these changes out. 774694 13 abarth 2012-11-23 13:49:02 -0800 Fascinating! This bot is now healed: http://build.chromium.org/f/chromium/perf/chromium-rel-win7-webkit/dromaeo_domcorequery/report.html?rev=169350&graph=dom_query_getElementsByTagName__not_in_document_&history=200 Here's what happened: 1) http://trac.webkit.org/changeset/135339 actually did fix the regression. We didn't see the bot healed because http://trac.webkit.org/changeset/135338 introduced a new regression. 2) http://trac.webkit.org/changeset/135440 rolled out http://trac.webkit.org/changeset/135338, and we are now able to observe that the bot is indeed healed. 175310 2012-11-20 17:18:10 -0800 2012-11-20 18:16:11 -0800 Patch bug-102852-20121120171558.patch text/plain 2178 abarth U3VidmVyc2lvbiBSZXZpc2lvbjogMTM1MzM0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggN2U4MTVlNmUwNjVmMTA0 YTc1MWFjMDU3ZTMyNjk4N2QwMDBjZmNiNC4uZDc2Yjg5ODNhZTQ1OWVjMjVjMjcyNjEzMzlmNDBl NjIzNWM5NmFmZCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDEyLTExLTIwICBBZGFt IEJhcnRoICA8YWJhcnRoQHdlYmtpdC5vcmc+CisKKyAgICAgICAgNyUgcmVncmVzc2lvbiBpbiBk cm9tYWVvX2RvbWNvcmVxdWVyeS9kb21fcXVlcnlfZ2V0RWxlbWVudHNCeVRhZ05hbWVfX25vdF9p bl9kb2N1bWVudAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/ aWQ9MTAyODUyCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAg ICAgSW4gaHR0cDovL3RyYWMud2Via2l0Lm9yZy9jaGFuZ2VzZXQvMTM1MjA4Lywgd2UgbG9zdCB0 aGUgZWFybHktZXhpdAorICAgICAgICBjaGVjayBmb3Igd2hldGhlciBpc29sYXRlZFdvcmxkc0V4 aXN0KCkgaW4gdGhpcyBjb2RlIHBhdGguIFRoaXMKKyAgICAgICAgcmVncmVzc2lvbiBwb2ludHMg dG8gYmVuY2htYXJrcyB0aGF0IHdlIGNhbiBmdXJ0aGVyIGltcHJvdmUgYnkKKyAgICAgICAgY29u dGludWluZyB0byBtZXJnZSB0aGUgZ2VuZXJhbCBTY3JpcHRXcmFwcGFibGUgY29kZSBwYXRoIHdp dGggdGhlIE5vZGUKKyAgICAgICAgY29kZSBwYXRoLgorCisgICAgICAgICogYmluZGluZ3Mvdjgv RE9NRGF0YVN0b3JlLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkRPTURhdGFTdG9yZTo6Y3VycmVu dCk6CisKIDIwMTItMTEtMjAgIEpvc2h1YSBCZWxsICA8anNiZWxsQGNocm9taXVtLm9yZz4KIAog ICAgICAgICBJbmRleGVkREI6IE1vdmUgY29udHJvbCBvZiB0cmFuc2FjdGlvbiBjb21wbGV0aW9u IHRvIGZyb250IGVuZApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvYmluZGluZ3MvdjgvRE9N RGF0YVN0b3JlLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL3Y4L0RPTURhdGFTdG9yZS5j cHAKaW5kZXggNDU2OTg3NTEzYTkzZGNkMmZjNjhjMWYzMTE0NTEwM2M2ZDc2MzZlZi4uNTg4OTdj NzczY2VhMDdjNGQ5YWI5OWQzN2I4N2NlYjIyYjM2MTgwZSAxMDA2NDQKLS0tIGEvU291cmNlL1dl YkNvcmUvYmluZGluZ3MvdjgvRE9NRGF0YVN0b3JlLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9i aW5kaW5ncy92OC9ET01EYXRhU3RvcmUuY3BwCkBAIC01NCwxMiArNTQsMTcgQEAgRE9NRGF0YVN0 b3JlOjp+RE9NRGF0YVN0b3JlKCkKIERPTURhdGFTdG9yZSogRE9NRGF0YVN0b3JlOjpjdXJyZW50 KHY4OjpJc29sYXRlKiBpc29sYXRlKQogewogICAgIERFRklORV9TVEFUSUNfTE9DQUwoRE9NRGF0 YVN0b3JlLCBtYWluV29ybGRET01EYXRhU3RvcmUsIChNYWluV29ybGQpKTsKKwogICAgIFY4UGVy SXNvbGF0ZURhdGEqIGRhdGEgPSBpc29sYXRlID8gVjhQZXJJc29sYXRlRGF0YTo6ZnJvbShpc29s YXRlKSA6IFY4UGVySXNvbGF0ZURhdGE6OmN1cnJlbnQoKTsKICAgICBpZiAoVU5MSUtFTFkoISFk YXRhLT5kb21EYXRhU3RvcmUoKSkpCiAgICAgICAgIHJldHVybiBkYXRhLT5kb21EYXRhU3RvcmUo KTsKLSAgICBWOERPTVdpbmRvd1NoZWxsKiBzaGVsbCA9IFY4RE9NV2luZG93U2hlbGw6Omlzb2xh dGVkKHY4OjpDb250ZXh0OjpHZXRFbnRlcmVkKCkpOwotICAgIGlmIChVTkxJS0VMWSghIXNoZWxs KSkKLSAgICAgICAgcmV0dXJuIHNoZWxsLT53b3JsZCgpLT5pc29sYXRlZFdvcmxkRE9NRGF0YVN0 b3JlKCk7CisKKyAgICBpZiAoRE9NV3JhcHBlcldvcmxkOjppc29sYXRlZFdvcmxkc0V4aXN0KCkp IHsKKyAgICAgICAgVjhET01XaW5kb3dTaGVsbCogc2hlbGwgPSBWOERPTVdpbmRvd1NoZWxsOjpp c29sYXRlZCh2ODo6Q29udGV4dDo6R2V0RW50ZXJlZCgpKTsKKyAgICAgICAgaWYgKFVOTElLRUxZ KCEhc2hlbGwpKQorICAgICAgICAgICAgcmV0dXJuIHNoZWxsLT53b3JsZCgpLT5pc29sYXRlZFdv cmxkRE9NRGF0YVN0b3JlKCk7CisgICAgfQorCiAgICAgcmV0dXJuICZtYWluV29ybGRET01EYXRh U3RvcmU7CiB9CiAK