Bug 200195

Summary: [JSC] WeakMapImpl do not need to take cellLock in visitOutputConstraints and main thread
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch mark.lam: review+

Yusuke Suzuki
Reported 2019-07-27 02:40:35 PDT
[JSC] WeakMapImpl do not need to take cellLock in visitOutputConstraint and main thread
Attachments
Patch (4.06 KB, patch)
2019-07-27 02:43 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (4.39 KB, patch)
2022-02-23 17:33 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (4.75 KB, patch)
2022-02-23 17:36 PST, Yusuke Suzuki 		mark.lam: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1 2019-07-27 02:43:22 PDT
Created attachment 375026 [details] Patch
Yusuke Suzuki
Comment 2 2022-02-23 17:33:03 PST
Created attachment 453059 [details] Patch
Yusuke Suzuki
Comment 3 2022-02-23 17:36:35 PST
Created attachment 453060 [details] Patch
Mark Lam
Comment 4 2022-02-23 19:08:08 PST
Comment on attachment 453060 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=453060&action=review r=me > Source/JavaScriptCore/ChangeLog:8 > + WeakMapImpl::visitOutputConstraints is called in the constraint solver, and it does not modify the buffer of WeakMapImpl. Can you remove this part: "and it does not modify the buffer of WeakMapImpl"? It is not relevant, correct? The only relevant detail is what you stated below i.e. the constraint solver runs and rehash can never be run concurrently. Also, if concurrency were an issue, m_buffer isn't the only thing that needs to be protected: m_capacity does too. So, it seems strange to draw attention to the buffer here.
Yusuke Suzuki
Comment 5 2022-02-23 20:23:04 PST
Comment on attachment 453060 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=453060&action=review >> Source/JavaScriptCore/ChangeLog:8 >> + WeakMapImpl::visitOutputConstraints is called in the constraint solver, and it does not modify the buffer of WeakMapImpl. > > Can you remove this part: "and it does not modify the buffer of WeakMapImpl"? It is not relevant, correct? The only relevant detail is what you stated below i.e. the constraint solver runs and rehash can never be run concurrently. > > Also, if concurrency were an issue, m_buffer isn't the only thing that needs to be protected: m_capacity does too. So, it seems strange to draw attention to the buffer here. OK, dropped.
Yusuke Suzuki
Comment 6 2022-02-24 00:57:20 PST
Committed r290416 (247724@trunk): <https://commits.webkit.org/247724@trunk>
Radar WebKit Bug Importer
Comment 7 2022-02-24 00:58:20 PST
<rdar://problem/89405434>
Note You need to log in before you can comment on or make changes to this bug.