CPU Usage: Worker thread that dies might stay at a high value forever Steps to Reproduce: 1. Inspect maps.google.com 2. Reload => If a Worker Thread is sampled high and then dies it may stay around forever at a high value Notes: - Need to kill off dead workers in the range
Created attachment 363196 [details] [PATCH] Proposed Fix
Comment on attachment 363196 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=363196&action=review r=me > Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.js:413 > + let deadWorkers = workersSeenLastRecord.difference(workersSeenCurrentRecord); Cool!
Comment on attachment 363196 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=363196&action=review > Source/WebInspectorUI/ChangeLog:10 > + Handle workers dieing or at least zeroing out between records. s/dieing/dying > Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.js:304 > + let workersSeenCurrentRecord = new Set; Having "record" makes me think that this has something to do with actual timeline records. I think a name without "record" in it (or one that explains more that the records are what you're filtering off of, like `workersWithRecentRecords`) would be better. > Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.js:366 > + let workersSeenLastRecord = workersSeenCurrentRecord; > + workersSeenCurrentRecord = new Set; Rather than try to do some fancy checking of usage numbers, couldn't you check with `WI.WorkerManager` and see if the `workerId` is still active? It seems to me that that's the only way of knowing whether a worker is "dead". Are you trying to make it so that alive workers with no activity don't appear, or that actually dead (`terminate`d) workers don't appear?
(In reply to Devin Rousso from comment #3) > Comment on attachment 363196 [details] > [PATCH] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=363196&action=review > > > Source/WebInspectorUI/ChangeLog:10 > > + Handle workers dieing or at least zeroing out between records. > > s/dieing/dying > > > Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.js:304 > > + let workersSeenCurrentRecord = new Set; > > Having "record" makes me think that this has something to do with actual > timeline records. I think a name without "record" in it (or one that > explains more that the records are what you're filtering off of, like > `workersWithRecentRecords`) would be better. I'll go with workersSeen*In*CurrentRecord or workersSeen*In*LastRecord > > > Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.js:366 > > + let workersSeenLastRecord = workersSeenCurrentRecord; > > + workersSeenCurrentRecord = new Set; > > Rather than try to do some fancy checking of usage numbers, couldn't you > check with `WI.WorkerManager` and see if the `workerId` is still active? It > seems to me that that's the only way of knowing whether a worker is "dead". > > Are you trying to make it so that alive workers with no activity don't > appear, or that actually dead (`terminate`d) workers don't appear? The timeline range selection might be a period of time in the past where a worker existed, but no longer exist. So checking WorkerManager/TargetManager could be insufficient. Dropping any worker thread to zero if not seen again is a safe way of handling this regardless of any outside data from the CPU Timeline Records themselves.
https://trac.webkit.org/r242243
<rdar://problem/48494364>