159569 2016-07-08 11:13:10 -0700 Dumping the object graph doesn't work with verbose GC logging 2016-07-08 11:57:15 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 msaboff msaboff commit-queue keith_miller mark.lam saam webkit-bug-importer oldest_to_newest 1209201 0 msaboff 2016-07-08 11:13:10 -0700 When the logGC option is set to verbose (2), the logging code attempts to dump the object graph. That code doesn't work. It is built to visit the object graph again and that method it fraught with difficulties. Therefore the object graph dumping part of verbose GC logging should be removed. 1209205 1 283184 msaboff 2016-07-08 11:21:05 -0700 Created attachment 283184 Patch 1209206 2 webkit-bug-importer 2016-07-08 11:21:51 -0700 <rdar://problem/27249686> 1209209 3 283184 mark.lam 2016-07-08 11:30:02 -0700 Comment on attachment 283184 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=283184&action=review > Source/JavaScriptCore/ChangeLog:11 > + The current object graph logging code tries to revisits the graph. This doesn't work > + correctly and asking around it isn't used. The only way to dump the true object graph > + is to log while we GC and that has obvious performance implications. > + Therefore I eliminated GCLogging::dumpObjectGraph() and related code. Then, you should also remove the verbose option from Options::logGC. 1209210 4 msaboff 2016-07-08 11:32:43 -0700 (In reply to comment #3) > Comment on attachment 283184 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=283184&action=review > > > Source/JavaScriptCore/ChangeLog:11 > > + The current object graph logging code tries to revisits the graph. This doesn't work > > + correctly and asking around it isn't used. The only way to dump the true object graph > > + is to log while we GC and that has obvious performance implications. > > + Therefore I eliminated GCLogging::dumpObjectGraph() and related code. > > Then, you should also remove the verbose option from Options::logGC. We still need the verbose option, which will show all of the roots. This patch only removes the dump object graph code. I still think that logging the roots is useful. 1209211 5 283184 mark.lam 2016-07-08 11:33:00 -0700 Comment on attachment 283184 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=283184&action=review r=me >> Source/JavaScriptCore/ChangeLog:11 >> + Therefore I eliminated GCLogging::dumpObjectGraph() and related code. > > Then, you should also remove the verbose option from Options::logGC. My mistake: the verbose option is still in use. 1209224 6 283184 commit-queue 2016-07-08 11:57:10 -0700 Comment on attachment 283184 Patch Clearing flags on attachment: 283184 Committed r203000: <http://trac.webkit.org/changeset/203000> 1209225 7 commit-queue 2016-07-08 11:57:15 -0700 All reviewed patches have been landed. Closing bug. 283184 2016-07-08 11:21:05 -0700 2016-07-08 11:57:10 -0700 Patch 159569.patch text/plain 4230 msaboff SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMjAyOTk0KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDI3IEBA CisyMDE2LTA3LTA4ICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgorCisgICAg ICAgIER1bXBpbmcgdGhlIG9iamVjdCBncmFwaCBkb2Vzbid0IHdvcmsgd2l0aCB2ZXJib3NlIEdD IGxvZ2dpbmcKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTE1OTU2OQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IFRoZSBjdXJyZW50IG9iamVjdCBncmFwaCBsb2dnaW5nIGNvZGUgdHJpZXMgdG8gcmV2aXNpdHMg dGhlIGdyYXBoLiAgVGhpcyBkb2Vzbid0IHdvcmsKKyAgICAgICAgY29ycmVjdGx5IGFuZCBhc2tp bmcgYXJvdW5kIGl0IGlzbid0IHVzZWQuICBUaGUgb25seSB3YXkgdG8gZHVtcCB0aGUgdHJ1ZSBv YmplY3QgZ3JhcGgKKyAgICAgICAgaXMgdG8gbG9nIHdoaWxlIHdlIEdDIGFuZCB0aGF0IGhhcyBv YnZpb3VzIHBlcmZvcm1hbmNlIGltcGxpY2F0aW9ucy4KKyAgICAgICAgVGhlcmVmb3JlIEkgZWxp bWluYXRlZCBHQ0xvZ2dpbmc6OmR1bXBPYmplY3RHcmFwaCgpIGFuZCByZWxhdGVkIGNvZGUuICAK KworICAgICAgICAqIGhlYXAvR0NMb2dnaW5nLmNwcDoKKyAgICAgICAgKEpTQzo6R0NMb2dnaW5n OjpsZXZlbEFzU3RyaW5nKToKKyAgICAgICAgKEpTQzo6TG9nZ2luZ0Z1bmN0b3I6OkxvZ2dpbmdG dW5jdG9yKTogRGVsZXRlZC4KKyAgICAgICAgKEpTQzo6TG9nZ2luZ0Z1bmN0b3I6On5Mb2dnaW5n RnVuY3Rvcik6IERlbGV0ZWQuCisgICAgICAgIChKU0M6OkxvZ2dpbmdGdW5jdG9yOjpvcGVyYXRv cigpKTogRGVsZXRlZC4KKyAgICAgICAgKEpTQzo6TG9nZ2luZ0Z1bmN0b3I6OmxvZyk6IERlbGV0 ZWQuCisgICAgICAgIChKU0M6OkxvZ2dpbmdGdW5jdG9yOjpyZXZpdmVDZWxscyk6IERlbGV0ZWQu CisgICAgICAgIChKU0M6OkxvZ2dpbmdGdW5jdG9yOjpyZXR1cm5WYWx1ZSk6IERlbGV0ZWQuCisg ICAgICAgIChKU0M6OkdDTG9nZ2luZzo6ZHVtcE9iamVjdEdyYXBoKTogRGVsZXRlZC4KKyAgICAg ICAgKiBoZWFwL0hlYXAuY3BwOgorICAgICAgICAoSlNDOjpIZWFwOjpkaWRGaW5pc2hDb2xsZWN0 aW9uKToKKwogMjAxNi0wNy0wOCAgTWFyayBMYW0gIDxtYXJrLmxhbUBhcHBsZS5jb20+CiAKICAg ICAgICAgUmVuYW1lIGpzQ1BVU3RhY2tMaW1pdCB0byBvc1N0YWNrTGltaXRXaXRoUmVzZXJ2ZSBh bmQganNFbXVsYXRlZFN0YWNrTGltaXQgdG8gY2xvb3BTdGFja0xpbWl0LgpJbmRleDogU291cmNl L0phdmFTY3JpcHRDb3JlL2hlYXAvR0NMb2dnaW5nLmNwcAo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2Uv SmF2YVNjcmlwdENvcmUvaGVhcC9HQ0xvZ2dpbmcuY3BwCShyZXZpc2lvbiAyMDI5NzcpCisrKyBT b3VyY2UvSmF2YVNjcmlwdENvcmUvaGVhcC9HQ0xvZ2dpbmcuY3BwCSh3b3JraW5nIGNvcHkpCkBA IC01MCw2OSArNTAsNiBAQCBjb25zdCBjaGFyKiBHQ0xvZ2dpbmc6OmxldmVsQXNTdHJpbmcoTGV2 CiAgICAgfQogfQogCi1jbGFzcyBMb2dnaW5nRnVuY3RvciB7Ci1wdWJsaWM6Ci0gICAgTG9nZ2lu Z0Z1bmN0b3IoU2xvdFZpc2l0b3ImIHNsb3RWaXNpdG9yKQotICAgICAgICA6IG1fc2xvdFZpc2l0 b3Ioc2xvdFZpc2l0b3IpCi0gICAgewotICAgICAgICBtX3NhdmVkTWFya1N0YWNrLnJlc2l6ZSht X3Nsb3RWaXNpdG9yLm1hcmtTdGFjaygpLnNpemUoKSk7Ci0gICAgICAgIG1fc2xvdFZpc2l0b3Iu bWFya1N0YWNrKCkuZmlsbFZlY3RvcihtX3NhdmVkTWFya1N0YWNrKTsKLSAgICB9Ci0KLSAgICB+ TG9nZ2luZ0Z1bmN0b3IoKQotICAgIHsKLSAgICAgICAgcmV2aXZlQ2VsbHMoKTsKLSAgICB9Ci0K LSAgICBJdGVyYXRpb25TdGF0dXMgb3BlcmF0b3IoKShKU0NlbGwqIGNlbGwpCi0gICAgewotICAg ICAgICBtX2xpdmVDZWxscy5hcHBlbmQoY2VsbCk7Ci0gICAgICAgIE1hcmtlZEJsb2NrOjpibG9j a0ZvcihjZWxsKS0+Y2xlYXJNYXJrZWQoY2VsbCk7Ci0gICAgICAgIHJldHVybiBJdGVyYXRpb25T dGF0dXM6OkNvbnRpbnVlOwotICAgIH0KLQotICAgIHZvaWQgbG9nKCkKLSAgICB7Ci0gICAgICAg IG1fc2xvdFZpc2l0b3IuY2xlYXJNYXJrU3RhY2soKTsKLSAgICAgICAgZm9yIChKU0NlbGwqIGNl bGwgOiBtX2xpdmVDZWxscykgewotICAgICAgICAgICAgY2VsbC0+bWV0aG9kVGFibGUoKS0+dmlz aXRDaGlsZHJlbihjZWxsLCBtX3Nsb3RWaXNpdG9yKTsKLSAgICAgICAgICAgIGRhdGFMb2coIlxu IiwgKmNlbGwsICI6XG4iLCBtX3Nsb3RWaXNpdG9yKTsKLSAgICAgICAgICAgIGZvciAoY29uc3Qg SlNDZWxsKiBuZWlnaGJvciA6IG1fc2xvdFZpc2l0b3IubWFya1N0YWNrKCkpCi0gICAgICAgICAg ICAgICAgTWFya2VkQmxvY2s6OmJsb2NrRm9yKG5laWdoYm9yKS0+Y2xlYXJNYXJrZWQobmVpZ2hi b3IpOwotICAgICAgICAgICAgbV9zbG90VmlzaXRvci5jbGVhck1hcmtTdGFjaygpOwotICAgICAg ICB9Ci0gICAgICAgIG1fc2xvdFZpc2l0b3IucmVzZXQoKTsKLSAgICB9Ci0KLSAgICB2b2lkIHJl dml2ZUNlbGxzKCkKLSAgICB7Ci0gICAgICAgIGZvciAoSlNDZWxsKiBjZWxsIDogbV9saXZlQ2Vs bHMpCi0gICAgICAgICAgICBNYXJrZWRCbG9jazo6YmxvY2tGb3IoY2VsbCktPnNldE1hcmtlZChj ZWxsKTsKLQotICAgICAgICBmb3IgKGNvbnN0IEpTQ2VsbCogY2VsbCA6IG1fc2F2ZWRNYXJrU3Rh Y2spIHsKLSAgICAgICAgICAgIG1fc2xvdFZpc2l0b3IubWFya1N0YWNrKCkuYXBwZW5kKGNlbGwp OwotICAgICAgICAgICAgY2VsbC0+c2V0Q2VsbFN0YXRlKENlbGxTdGF0ZTo6T2xkR3JleSk7Ci0g ICAgICAgIH0KLSAgICB9Ci0KLSAgICB0eXBlZGVmIHZvaWQgUmV0dXJuVHlwZTsKLQotICAgIHZv aWQgcmV0dXJuVmFsdWUoKSB7IH07Ci0KLXByaXZhdGU6Ci0gICAgVmVjdG9yPGNvbnN0IEpTQ2Vs bCo+IG1fc2F2ZWRNYXJrU3RhY2s7Ci0gICAgVmVjdG9yPEpTQ2VsbCo+IG1fbGl2ZUNlbGxzOwot ICAgIFNsb3RWaXNpdG9yJiBtX3Nsb3RWaXNpdG9yOwotfTsKLQotdm9pZCBHQ0xvZ2dpbmc6OmR1 bXBPYmplY3RHcmFwaChIZWFwKiBoZWFwKQotewotICAgIExvZ2dpbmdGdW5jdG9yIGxvZ2dpbmdG dW5jdG9yKGhlYXAtPm1fc2xvdFZpc2l0b3IpOwotICAgIEhlYXBJdGVyYXRpb25TY29wZSBpdGVy YXRpb25TY29wZSgqaGVhcCk7Ci0gICAgaGVhcC0+b2JqZWN0U3BhY2UoKS5mb3JFYWNoTGl2ZUNl bGwoaXRlcmF0aW9uU2NvcGUsIGxvZ2dpbmdGdW5jdG9yKTsKLSAgICBsb2dnaW5nRnVuY3Rvci5s b2coKTsKLX0KLQogfSAvLyBuYW1lc3BhY2UgSlNDCiAKIG5hbWVzcGFjZSBXVEYgewpJbmRleDog U291cmNlL0phdmFTY3JpcHRDb3JlL2hlYXAvSGVhcC5jcHAKPT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNl L0phdmFTY3JpcHRDb3JlL2hlYXAvSGVhcC5jcHAJKHJldmlzaW9uIDIwMjk3NykKKysrIFNvdXJj ZS9KYXZhU2NyaXB0Q29yZS9oZWFwL0hlYXAuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0xNDYzLDkg KzE0NjMsNiBAQCB2b2lkIEhlYXA6OmRpZEZpbmlzaENvbGxlY3Rpb24oZG91YmxlIGdjCiAgICAg aWYgKE9wdGlvbnM6OmR1bXBPYmplY3RTdGF0aXN0aWNzKCkpCiAgICAgICAgIEhlYXBTdGF0aXN0 aWNzOjpkdW1wT2JqZWN0U3RhdGlzdGljcyh0aGlzKTsKIAotICAgIGlmIChPcHRpb25zOjpsb2dH QygpID09IEdDTG9nZ2luZzo6VmVyYm9zZSkKLSAgICAgICAgR0NMb2dnaW5nOjpkdW1wT2JqZWN0 R3JhcGgodGhpcyk7Ci0KICAgICBpZiAoSGVhcFByb2ZpbGVyKiBoZWFwUHJvZmlsZXIgPSBtX3Zt LT5oZWFwUHJvZmlsZXIoKSkgewogICAgICAgICBnYXRoZXJFeHRyYUhlYXBTbmFwc2hvdERhdGEo KmhlYXBQcm9maWxlcik7CiAgICAgICAgIHJlbW92ZURlYWRIZWFwU25hcHNob3ROb2RlcygqaGVh cFByb2ZpbGVyKTsK