51479 2010-12-22 10:10:49 -0800 Web Inspector: chromium: inspector's tests are quite slow especially at windows(Debug). 2011-01-04 11:16:54 -0800 1 1 1 Unclassified WebKit Web Inspector (Deprecated) 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 loislo loislo apavlov bweinstein commit-queue eric joepeck keishi loislo pfeldman pmuellr rik timothy tkent tony yurys oldest_to_newest 325541 0 loislo 2010-12-22 10:10:49 -0800 almost all inspector's tests become flaky on chromium Win(dbg) after switching to DRT. I just discovered that we have two additional GC runs per each WebViewHost. Test_shell version had no such calls. These gc runs eat 15% of cpu time. 325545 1 77228 loislo 2010-12-22 10:13:59 -0800 Created attachment 77228 [patch] initial version 325550 2 joepeck 2010-12-22 10:23:15 -0800 This isn't my territory, but a bit of background can be found on: http://trac.webkit.org/changeset/71928 > In test_shell, when we close a window, we first load about:blank > and run GC to fire the destruction logic of the page. In test_shell, > this happens in ~TestShell. In DRT, we manage virtual windows as > WebViewHosts, so we need to replicate this logic in ~WebViewHost. 325608 3 loislo 2010-12-22 11:43:29 -0800 (In reply to comment #2) > This isn't my territory, but a bit of background can be found on: > http://trac.webkit.org/changeset/71928 > > > In test_shell, when we close a window, we first load about:blank > > and run GC to fire the destruction logic of the page. In test_shell, > > this happens in ~TestShell. In DRT, we manage virtual windows as > > WebViewHosts, so we need to replicate this logic in ~WebViewHost. I may be wrong but historically there was only one TestShell object per test_shell process. I added another instance of TestShell object for the Inspector's window when I did inspector's tests support in test_shell. I think that is the reason why we had a gc run for each inspector tests in test_shell. Right now in chromium version of DRT we have double gc run for each layout test and four gc runs for each inspector test. I think this is overkill. 325971 4 77228 eric 2010-12-23 01:12:56 -0800 Comment on attachment 77228 [patch] initial version Do we know when these were added or why? 325974 5 eric 2010-12-23 01:14:46 -0800 Unfortunately Tony is on vacation, otherwise I would really like to see his commentary here. I guess we can try this for now and roll this out if he comes back and can explain why it's needed. We have general timeout/crash trouble with inspector tests. See my mail from an hour ago to webkit-dev about crashy inspector tests. 325975 6 77228 eric 2010-12-23 01:15:14 -0800 Comment on attachment 77228 [patch] initial version rs=me. 325980 7 77228 loislo 2010-12-23 01:20:55 -0800 Comment on attachment 77228 [patch] initial version Clearing flags on attachment: 77228 Committed r74544: <http://trac.webkit.org/changeset/74544> 325981 8 loislo 2010-12-23 01:21:05 -0800 All reviewed patches have been landed. Closing bug. 329097 9 tony 2011-01-04 11:16:54 -0800 (In reply to comment #3) > (In reply to comment #2) > > This isn't my territory, but a bit of background can be found on: > > http://trac.webkit.org/changeset/71928 > > > > > In test_shell, when we close a window, we first load about:blank > > > and run GC to fire the destruction logic of the page. In test_shell, > > > this happens in ~TestShell. In DRT, we manage virtual windows as > > > WebViewHosts, so we need to replicate this logic in ~WebViewHost. > > I may be wrong but historically there was only one TestShell object per test_shell process. > I added another instance of TestShell object for the Inspector's window when I did inspector's tests support in test_shell. In test_shell, there's one TestShell per window. In DRT, there's only one TestShell instance, so I moved the gc calls into ~WebViewHost to mirror the usage in test_shell. I think there might have been some plugin tests that depended on this behavior, but if there were no test failures after this change, seems fine to me. 77228 2010-12-22 10:13:59 -0800 2010-12-23 01:20:55 -0800 [patch] initial version patch text/plain 1435 loislo ZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBiL1Rvb2xzL0NoYW5nZUxvZwppbmRleCBhOGUw OTFlLi42ZjlhYWVjIDEwMDY0NAotLS0gYS9Ub29scy9DaGFuZ2VMb2cKKysrIGIvVG9vbHMvQ2hh bmdlTG9nCkBAIC0xLDUgKzEsMjEgQEAKIDIwMTAtMTItMjIgIElseWEgVGlraG9ub3Zza3kgIDxs b2lzbG9AY2hyb21pdW0ub3JnPgogCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEp LgorCisgICAgICAgIFdlYiBJbnNwZWN0b3I6IGNocm9taXVtOiBpbnNwZWN0b3IncyB0ZXN0cyBh cmUgcXVpdGUgc2xvdyBlc3BlY2lhbGx5IGF0IHdpbmRvd3MoRGVidWcpLgorCisgICAgICAgIEFs bW9zdCBhbGwgaW5zcGVjdG9yJ3MgdGVzdHMgYmVjb21lIGZsYWt5IG9uIGNocm9taXVtIFdpbihk YmcpIGFmdGVyIHN3aXRjaGluZyB0byBEUlQuCisgICAgICAgIEkganVzdCBkaXNjb3ZlcmVkIHRo YXQgd2UgaGF2ZSB0d28gYWRkaXRpb25hbCBHQyBydW5zIHBlciBlYWNoIFdlYlZpZXdIb3N0Lgor ICAgICAgICBUZXN0X3NoZWxsIGhhZCBubyBzdWNoIGNhbGxzLgorICAgICAgICBUaGVzZSBnYyBy dW5zIGVhdCAxNSUgb2YgY3B1IHRpbWUuCisKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5v cmcvc2hvd19idWcuY2dpP2lkPTUxNDc5CisKKyAgICAgICAgKiBEdW1wUmVuZGVyVHJlZS9jaHJv bWl1bS9XZWJWaWV3SG9zdC5jcHA6CisgICAgICAgIChXZWJWaWV3SG9zdDo6fldlYlZpZXdIb3N0 KToKKworMjAxMC0xMi0yMiAgSWx5YSBUaWtob25vdnNreSAgPGxvaXNsb0BjaHJvbWl1bS5vcmc+ CisKICAgICAgICAgUmV2aWV3ZWQgYnkgWXVyeSBTZW1pa2hhdHNreS4KIAogICAgICAgICBXZWIg SW5zcGVjdG9yOiBbY2hyb21pdW1dIERSVCBpbnNwZWN0b3IgbGF5b3V0IHRlc3RzIGFyZSBmbGFr eSBpbiBkZWJ1Zy4KZGlmZiAtLWdpdCBhL1Rvb2xzL0R1bXBSZW5kZXJUcmVlL2Nocm9taXVtL1dl YlZpZXdIb3N0LmNwcCBiL1Rvb2xzL0R1bXBSZW5kZXJUcmVlL2Nocm9taXVtL1dlYlZpZXdIb3N0 LmNwcAppbmRleCBlM2M4YzI4Li5kODgwODc5IDEwMDY0NAotLS0gYS9Ub29scy9EdW1wUmVuZGVy VHJlZS9jaHJvbWl1bS9XZWJWaWV3SG9zdC5jcHAKKysrIGIvVG9vbHMvRHVtcFJlbmRlclRyZWUv Y2hyb21pdW0vV2ViVmlld0hvc3QuY3BwCkBAIC0xMTMzLDEwICsxMTMzLDYgQEAgV2ViVmlld0hv c3Q6On5XZWJWaWV3SG9zdCgpCiAgICAgICAgIGxvYWRVUkxGb3JGcmFtZShHVVJMKCJhYm91dDpi bGFuayIpLCBXZWJTdHJpbmcoKSk7CiAgICAgfQogCi0gICAgLy8gQ2FsbCBHQyB0d2ljZSB0byBj bGVhbiB1cCBnYXJiYWdlLgotICAgIG1fc2hlbGwtPmNhbGxKU0dDKCk7Ci0gICAgbV9zaGVsbC0+ Y2FsbEpTR0MoKTsKLQogICAgIHdlYldpZGdldCgpLT5jbG9zZSgpOwogCiAgICAgaWYgKG1faW5N b2RhbExvb3ApCg==