RESOLVED FIXED Bug 225676
ConservativeRoots triggers page demand on Speedometer 
https://bugs.webkit.org/show_bug.cgi?id=225676
Summary ConservativeRoots triggers page demand on Speedometer
Geoffrey Garen
Reported 2021-05-11 15:54:03 PDT
ConservativeRoots triggers page demand on Speedometer
Attachments
Patch (5.64 KB, patch)
2021-05-11 15:57 PDT, Geoffrey Garen 		no flags
DetailsFormatted DiffDiff
Patch (2.30 KB, patch)
2021-05-12 12:56 PDT, Geoffrey Garen 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Geoffrey Garen
Comment 1 2021-05-11 15:57:57 PDT
Created attachment 428319 [details] Patch
Geoffrey Garen
Comment 2 2021-05-11 17:21:45 PDT
Comment on attachment 428319 [details] Patch cq+
EWS
Comment 3 2021-05-11 17:52:18 PDT
Committed r277346 (237604@main): <https://commits.webkit.org/237604@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 428319 [details].
Radar WebKit Bug Importer
Comment 4 2021-05-11 17:53:16 PDT
<rdar://problem/77872621>
Filip Pizlo
Comment 5 2021-05-12 08:40:18 PDT
Comment on attachment 428319 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=428319&action=review > Source/JavaScriptCore/heap/ConservativeRoots.cpp:61 > + m_roots.append(bitwise_cast<HeapCell*>(p)); Doesn't this code while some threads are thread_suspended? In that case, I think you just introduced one of my favorite deadlocks: - The append() calls malloc, which tries to grab some lock. - Meanwhile some other thread is thread_suspended while holding that lock. Nothing says that thread_suspend can't stop a thread while it's inside fastMalloc, system malloc, or anything else that holds locks. I think that's why this code previously used a super janky want of allocating memory -- it was that way so that it didn't have to take locks to allocate.
WebKit Commit Bot
Comment 6 2021-05-12 11:32:35 PDT
Re-opened since this is blocked by bug 225705
Geoffrey Garen
Comment 7 2021-05-12 12:56:16 PDT
Created attachment 428405 [details] Patch
Geoffrey Garen
Comment 8 2021-05-12 13:14:31 PDT
Comment on attachment 428405 [details] Patch cq+
EWS
Comment 9 2021-05-12 14:06:43 PDT
Committed r277388 (237644@main): <https://commits.webkit.org/237644@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 428405 [details].
Note You need to log in before you can comment on or make changes to this bug.