150065 2015-10-12 17:20:39 -0700 REGRESSION: Web Inspector hangs for many seconds when trying to reload page 2015-10-14 20:54:29 -0700 1 1 1 Unclassified WebKit Web Inspector WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 1 joepeck joepeck bburg commit-queue ggaren graouts joepeck mark.lam mattbaker nvasilyev saam timothy webkit-bug-importer oldest_to_newest 1132593 0 joepeck 2015-10-12 17:20:39 -0700 * SUMMARY Inspector hangs for many seconds when trying to reload page. * STEPS TO REPRODUCE 1. Inspect cnn.com 2. Show Timeline tab 3. Reload => hangs for quite a while, may never show data * NOTES - Samples show lots of time spent under Inspector::ScriptDebugServer::sourceParsed (ContentSearchUtilities) - Seems like this may have been exposed by these recent changes: - <https://webkit.org/b/148347> REGRESSION(r188792): broke lots of tests, ggaren is going to investigate and reland (Requested by thorton on #webkit). - <https://webkit.org/b/148280> Unify code paths for manually deleting all code * RELATED: - We could eliminate the inspector's regex searching by having the JS Parser parse SourceURL / SourceURLMapping comments. This information would also be beneficial for expiation messages in the engine itself (e.g. exceptions in eval code). 1132594 1 webkit-bug-importer 2015-10-12 17:21:53 -0700 <rdar://problem/23081124> 1132597 2 joepeck 2015-10-12 17:26:12 -0700 It sounds like each time a Debugger is attached to a JSGlobalObject it will send sourceParsed for each source provider. Seems like we could be getting duplicate messages then. Since on a Web Page the same Debugger is shared across all Frames / Isolated Worlds. 1132682 3 joepeck 2015-10-12 20:04:03 -0700 (In reply to comment #2) > It sounds like each time a Debugger is attached to a JSGlobalObject it will > send sourceParsed for each source provider. Seems like we could be getting > duplicate messages then. Since on a Web Page the same Debugger is shared > across all Frames / Isolated Worlds. Yes, in the case of Safari on OS X as many as 6 or more isolated worlds get debuggers attached when loading a page (Inspector, Reader, Autofill, Extensions). In the case of the debugger being attached to each of these the inspector was re-pushing all scripts seen so far each time. In general I think there are improvements to be made here, but reducing this to just once (the first time a Debugger is attached) brings this back to what looks like original performance. 1132684 4 262969 joepeck 2015-10-12 20:23:40 -0700 Created attachment 262969 [PATCH] Proposed Fix 1132869 5 262969 bburg 2015-10-13 12:37:27 -0700 Comment on attachment 262969 [PATCH] Proposed Fix I suppose we can't test this because of the reloadPage issues in inspector tests? 1132951 6 joepeck 2015-10-13 16:10:05 -0700 (In reply to comment #5) > Comment on attachment 262969 [details] > [PATCH] Proposed Fix > > I suppose we can't test this because of the reloadPage issues in inspector > tests? I treated this as a performance issue, so I didn't write a test for that. But now that you mention it, I would be weary of testing for the reload page issues. It would be worth writing tests to ensure the frontend gets a list of all scripts when the inspector _opens_. 1133353 7 262969 mark.lam 2015-10-14 19:45:08 -0700 Comment on attachment 262969 [PATCH] Proposed Fix r=me 1133366 8 262969 commit-queue 2015-10-14 20:54:25 -0700 Comment on attachment 262969 [PATCH] Proposed Fix Clearing flags on attachment: 262969 Committed r191081: <http://trac.webkit.org/changeset/191081> 1133367 9 commit-queue 2015-10-14 20:54:29 -0700 All reviewed patches have been landed. Closing bug. 262969 2015-10-12 20:23:40 -0700 2015-10-14 20:54:25 -0700 [PATCH] Proposed Fix scripts.patch text/plain 2847 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvSmF2 YVNjcmlwdENvcmUvQ2hhbmdlTG9nCmluZGV4IDU5ZjgwZmQuLmIxYTc2OWUgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL0phdmFTY3JpcHRD b3JlL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDIyIEBACiAyMDE1LTEwLTEyICBKb3NlcGggUGVjb3Jh cm8gIDxwZWNvcmFyb0BhcHBsZS5jb20+CiAKKyAgICAgICAgUkVHUkVTU0lPTjogV2ViIEluc3Bl Y3RvciBoYW5ncyBmb3IgbWFueSBzZWNvbmRzIHdoZW4gdHJ5aW5nIHRvIHJlbG9hZCBwYWdlCisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNTAwNjUKKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBXaGVuIGRlYnVn Z2luZyBXZWIgUGFnZXMsIHRoZSBzYW1lIERlYnVnZ2VyIChQYWdlU2NyaXB0RGVidWdTZXJ2ZXIp IGlzCisgICAgICAgIGF0dGFjaGVkIHRvIGVhY2ggb2YgdGhlIGRpZmZlcmVudCBKU0dsb2JhbE9i amVjdHMgb24gdGhlIHBhZ2UuIFRoaXMgY291bGQKKyAgICAgICAgbWVhbiBtdWx0aXBsZSBmcmFt ZXMgb3IgaXNvbGF0ZWQgc2NyaXB0aW5nIGNvbnRleHRzLiBUaGVyZWZvcmUgd2Ugc2hvdWxkCisg ICAgICAgIG9ubHkgbmVlZCB0byBzZW5kIHNvdXJjZVBhcnNlZCBldmVudHMgdG8gdGhlIGZyb250 ZW5kIGZvciBzY3JpcHRzIHdpdGhpbgorICAgICAgICB0aGlzIG5ldyBKU0dsb2JhbE9iamVjdCwg bm90IGFueSBKU0dsb2JhbE9iamVjdCB0aGF0IGhhcyB0aGlzIGRlYnVnZ2VyLgorCisgICAgICAg ICogZGVidWdnZXIvRGVidWdnZXIuY3BwOgorICAgICAgICAoSlNDOjpEZWJ1Z2dlcjo6YXR0YWNo KToKKyAgICAgICAgT25seSBzZW5kIHNvdXJjZVBhcnNlZCBldmVudHMgZm9yIFNjcmlwdHMgaW4g dGhpcyBKU0dsb2JhbE9iamVjdC4KKworMjAxNS0xMC0xMiAgSm9zZXBoIFBlY29yYXJvICA8cGVj b3Jhcm9AYXBwbGUuY29tPgorCiAgICAgICAgIFNpbXBsaWZ5IFdlYWtCbG9jayB2aXNpdCBhbmQg cmVhcCBwaGFzZXMKICAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTE1MDA0NQogCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvZGVidWdnZXIv RGVidWdnZXIuY3BwIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL2RlYnVnZ2VyL0RlYnVnZ2VyLmNw cAppbmRleCA1MTQwMzhkLi4wN2RjYzgyIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvZGVidWdnZXIvRGVidWdnZXIuY3BwCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9kZWJ1 Z2dlci9EZWJ1Z2dlci5jcHAKQEAgLTQxLDEwICs0MSwxMCBAQCB1c2luZyBuYW1lc3BhY2UgSlND OwogCiBzdHJ1Y3QgR2F0aGVyU291cmNlUHJvdmlkZXJzIDogcHVibGljIE1hcmtlZEJsb2NrOjpW b2lkRnVuY3RvciB7CiAgICAgSGFzaFNldDxTb3VyY2VQcm92aWRlcio+IHNvdXJjZVByb3ZpZGVy czsKLSAgICBKU0M6OkRlYnVnZ2VyKiBtX2RlYnVnZ2VyOworICAgIEpTR2xvYmFsT2JqZWN0KiBt X2dsb2JhbE9iamVjdDsKIAotICAgIEdhdGhlclNvdXJjZVByb3ZpZGVycyhKU0M6OkRlYnVnZ2Vy KiBkZWJ1Z2dlcikKLSAgICAgICAgOiBtX2RlYnVnZ2VyKGRlYnVnZ2VyKSB7IH0KKyAgICBHYXRo ZXJTb3VyY2VQcm92aWRlcnMoSlNHbG9iYWxPYmplY3QqIGdsb2JhbE9iamVjdCkKKyAgICAgICAg OiBtX2dsb2JhbE9iamVjdChnbG9iYWxPYmplY3QpIHsgfQogCiAgICAgSXRlcmF0aW9uU3RhdHVz IG9wZXJhdG9yKCkoSlNDZWxsKiBjZWxsKQogICAgIHsKQEAgLTUyLDcgKzUyLDcgQEAgc3RydWN0 IEdhdGhlclNvdXJjZVByb3ZpZGVycyA6IHB1YmxpYyBNYXJrZWRCbG9jazo6Vm9pZEZ1bmN0b3Ig ewogICAgICAgICBpZiAoIWZ1bmN0aW9uKQogICAgICAgICAgICAgcmV0dXJuIEl0ZXJhdGlvblN0 YXR1czo6Q29udGludWU7CiAKLSAgICAgICAgaWYgKGZ1bmN0aW9uLT5zY29wZSgpLT5nbG9iYWxP YmplY3QoKS0+ZGVidWdnZXIoKSAhPSBtX2RlYnVnZ2VyKQorICAgICAgICBpZiAoZnVuY3Rpb24t PnNjb3BlKCktPmdsb2JhbE9iamVjdCgpICE9IG1fZ2xvYmFsT2JqZWN0KQogICAgICAgICAgICAg cmV0dXJuIEl0ZXJhdGlvblN0YXR1czo6Q29udGludWU7CiAKICAgICAgICAgaWYgKCFmdW5jdGlv bi0+ZXhlY3V0YWJsZSgpLT5pc0Z1bmN0aW9uRXhlY3V0YWJsZSgpKQpAQCAtMTQzLDggKzE0Myw4 IEBAIHZvaWQgRGVidWdnZXI6OmF0dGFjaChKU0dsb2JhbE9iamVjdCogZ2xvYmFsT2JqZWN0KQog ICAgIGdsb2JhbE9iamVjdC0+c2V0RGVidWdnZXIodGhpcyk7CiAgICAgbV9nbG9iYWxPYmplY3Rz LmFkZChnbG9iYWxPYmplY3QpOwogCi0gICAgLy8gQ2FsbCBzb3VyY2VQYXJzZWQoKSBiZWNhdXNl IGl0IHdpbGwgZXhlY3V0ZSBKYXZhU2NyaXB0IGluIHRoZSBpbnNwZWN0b3IuCi0gICAgR2F0aGVy U291cmNlUHJvdmlkZXJzIGdhdGhlclNvdXJjZVByb3ZpZGVycyh0aGlzKTsKKyAgICAvLyBDYWxs IHNvdXJjZVBhcnNlZCBiZWNhdXNlIGl0IHdpbGwgZXhlY3V0ZSBKYXZhU2NyaXB0IGluIHRoZSBp bnNwZWN0b3IuCisgICAgR2F0aGVyU291cmNlUHJvdmlkZXJzIGdhdGhlclNvdXJjZVByb3ZpZGVy cyhnbG9iYWxPYmplY3QpOwogICAgIHsKICAgICAgICAgSGVhcEl0ZXJhdGlvblNjb3BlIGl0ZXJh dGlvblNjb3BlKG1fdm0uaGVhcCk7CiAgICAgICAgIG1fdm0uaGVhcC5vYmplY3RTcGFjZSgpLmZv ckVhY2hMaXZlQ2VsbChpdGVyYXRpb25TY29wZSwgZ2F0aGVyU291cmNlUHJvdmlkZXJzKTsK