194111 2019-01-31 13:44:35 -0800 Web Inspector: Memory timeline starts from zero when both CPU and Memory timeline are enabled 2019-02-01 13:45:09 -0800 1 1 1 Unclassified WebKit Web Inspector WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 1 joepeck joepeck hi inspector-bugzilla-changes joepeck webkit-bug-importer oldest_to_newest 1500756 0 joepeck 2019-01-31 13:44:35 -0800 Memory timeline starts from zero when both CPU and Memory timeline are enabled Steps to Reproduce: 1. Inspect this page 2. Show CPU and Memory Timelines 3. Reload page => Memory starts from zero Notes: - The ResourceUsageThread starts immediately when the first listener (CPU) is added therefore misses values for the second listener (Memory) until the second sample 500ms later. We should start with a small wait to give multiple unique listeners a chance to register before the first sample. 1500757 1 webkit-bug-importer 2019-01-31 13:44:54 -0800 <rdar://problem/47714555> 1500820 2 360786 joepeck 2019-01-31 14:38:37 -0800 Created attachment 360786 [PATCH] Proposed Fix 1500892 3 360786 hi 2019-01-31 16:43:48 -0800 Comment on attachment 360786 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=360786&action=review rs=me > Source/WebCore/page/ResourceUsageThread.cpp:85 > + WTF::sleep(10_ms); Where did the `10_ms` come from? Is there any justification as to why that value was chosen? > Source/WebCore/page/ResourceUsageThread.cpp:127 > + WTF::sleep(10_ms); Should we also be waiting in the case that we already have observers? 1501204 4 joepeck 2019-02-01 11:54:57 -0800 (In reply to Devin Rousso from comment #3) > Comment on attachment 360786 [details] > [PATCH] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=360786&action=review > > rs=me > > > Source/WebCore/page/ResourceUsageThread.cpp:85 > > + WTF::sleep(10_ms); > > Where did the `10_ms` come from? Is there any justification as to why that > value was chosen? Nope, the sample rate is 500ms, so this is just a small pause. I do not have a good reason for 10, other then it is small. And allows enough time for other listeners to be activated. > > Source/WebCore/page/ResourceUsageThread.cpp:127 > > + WTF::sleep(10_ms); > > Should we also be waiting in the case that we already have observers? That is what `waitUntilObservers` does. If we waited in there we will have this 10ms pause. 1501229 5 360786 hi 2019-02-01 13:09:02 -0800 Comment on attachment 360786 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=360786&action=review >>> Source/WebCore/page/ResourceUsageThread.cpp:127 >>> + WTF::sleep(10_ms); >> >> Should we also be waiting in the case that we already have observers? > > That is what `waitUntilObservers` does. If we waited in there we will have this 10ms pause. I meant more of "why are we also waiting here if we're already waiting inside `waitUntilObservers`?" 1501241 6 joepeck 2019-02-01 13:26:27 -0800 https://trac.webkit.org/changeset/240868/webkit 1501261 7 360786 joepeck 2019-02-01 13:45:09 -0800 Comment on attachment 360786 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=360786&action=review >>>> Source/WebCore/page/ResourceUsageThread.cpp:127 >>>> + WTF::sleep(10_ms); >>> >>> Should we also be waiting in the case that we already have observers? >> >> That is what `waitUntilObservers` does. If we waited in there we will have this 10ms pause. > > I meant more of "why are we also waiting here if we're already waiting inside `waitUntilObservers`?" This is the first time we ever wake up the thread, and we wait here because it is created when we have a listener so it won't wait inside of `waitUntilObservers`. If all observers unregister the thread stays alive in its while loop and will wake up when new listeners are added, those will then wait inside of `waitUntilObservers`. 360786 2019-01-31 14:38:37 -0800 2019-02-01 11:55:09 -0800 [PATCH] Proposed Fix memory-1.patch text/plain 1940 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCAwODNiYjNkMGRmZC4uZTI4ZWE3Nzc3OTIgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxOCBAQAorMjAxOS0wMS0zMSAgSm9zZXBoIFBlY29yYXJvICA8cGVjb3Jhcm9AYXBwbGUu Y29tPgorCisgICAgICAgIFdlYiBJbnNwZWN0b3I6IE1lbW9yeSB0aW1lbGluZSBzdGFydHMgZnJv bSB6ZXJvIHdoZW4gYm90aCBDUFUgYW5kIE1lbW9yeSB0aW1lbGluZSBhcmUgZW5hYmxlZAorICAg ICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTk0MTExCisgICAg ICAgIDxyZGFyOi8vcHJvYmxlbS80NzcxNDU1NT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JP RFkgKE9PUFMhKS4KKworICAgICAgICAqIHBhZ2UvUmVzb3VyY2VVc2FnZVRocmVhZC5jcHA6Cisg ICAgICAgIChXZWJDb3JlOjpSZXNvdXJjZVVzYWdlVGhyZWFkOjp3YWl0VW50aWxPYnNlcnZlcnMp OgorICAgICAgICAoV2ViQ29yZTo6UmVzb3VyY2VVc2FnZVRocmVhZDo6dGhyZWFkQm9keSk6Cisg ICAgICAgIFdhaXQgYSBzaG9ydCBwZXJpb2Qgb2YgdGltZSBiZWZvcmUgdGhlIGZpcnN0IGxpc3Rl bmVyIHJlZ2lzdGVycworICAgICAgICBhbmQgd2Ugc3RhcnQgc2FtcGxpbmcuIFRoaXMgd2lsbCBh bGxvdyBtdWx0aXBsZSBsaXN0ZW5lcnMgdG8KKyAgICAgICAgcmVnaXN0ZXIsIGVhY2ggdGhhdCB3 YW50cyBkaWZmZXJlbnQgZGF0YS4KKwogMjAxOS0wMS0zMSAgRGFyaW4gQWRsZXIgIDxkYXJpbkBh cHBsZS5jb20+CiAKICAgICAgICAgU2ltcGxpZnkgYW5kIHN0cmVhbWxpbmUgY29kZSB0aGF0IGNy ZWF0ZXMgYW4gYXBwcm9wcmlhdGUgZG9jdW1lbnQgYmFzZWQgb24gTUlNRSB0eXBlCmRpZmYgLS1n aXQgYS9Tb3VyY2UvV2ViQ29yZS9wYWdlL1Jlc291cmNlVXNhZ2VUaHJlYWQuY3BwIGIvU291cmNl L1dlYkNvcmUvcGFnZS9SZXNvdXJjZVVzYWdlVGhyZWFkLmNwcAppbmRleCBkOWRkM2U3NjllZC4u NTAzNjI2MGM5YzQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BhZ2UvUmVzb3VyY2VVc2Fn ZVRocmVhZC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGFnZS9SZXNvdXJjZVVzYWdlVGhyZWFk LmNwcApAQCAtNzgsOCArNzgsMTIgQEAgdm9pZCBSZXNvdXJjZVVzYWdlVGhyZWFkOjpyZW1vdmVP YnNlcnZlcih2b2lkKiBrZXkpCiB2b2lkIFJlc291cmNlVXNhZ2VUaHJlYWQ6OndhaXRVbnRpbE9i c2VydmVycygpCiB7CiAgICAgTG9ja0hvbGRlciBsb2NrZXIobV9sb2NrKTsKLSAgICB3aGlsZSAo bV9vYnNlcnZlcnMuaXNFbXB0eSgpKQorICAgIHdoaWxlIChtX29ic2VydmVycy5pc0VtcHR5KCkp IHsKICAgICAgICAgbV9jb25kaXRpb24ud2FpdChtX2xvY2spOworCisgICAgICAgIC8vIFdhaXQg YSBiaXQgYWZ0ZXIgd2FraW5nIHVwIGZvciB0aGUgZmlyc3QgdGltZS4KKyAgICAgICAgV1RGOjpz bGVlcCgxMF9tcyk7CisgICAgfQogfQogCiB2b2lkIFJlc291cmNlVXNhZ2VUaHJlYWQ6Om5vdGlm eU9ic2VydmVycyhSZXNvdXJjZVVzYWdlRGF0YSYmIGRhdGEpCkBAIC0xMTksNiArMTIzLDkgQEAg dm9pZCBSZXNvdXJjZVVzYWdlVGhyZWFkOjpjcmVhdGVUaHJlYWRJZk5lZWRlZCgpCiAKIE5PX1JF VFVSTiB2b2lkIFJlc291cmNlVXNhZ2VUaHJlYWQ6OnRocmVhZEJvZHkoKQogeworICAgIC8vIFdh aXQgYSBiaXQgYWZ0ZXIgd2FraW5nIHVwIGZvciB0aGUgZmlyc3QgdGltZS4KKyAgICBXVEY6OnNs ZWVwKDEwX21zKTsKKyAgICAKICAgICB3aGlsZSAodHJ1ZSkgewogICAgICAgICAvLyBPbmx5IGRv IHdvcmsgaWYgd2UgaGF2ZSBvYnNlcnZlcnMuCiAgICAgICAgIHdhaXRVbnRpbE9ic2VydmVycygp Owo=