Bug 200195 - [JSC] WeakMapImpl do not need to take cellLock in visitOutputConstraints and main thread
Summary: [JSC] WeakMapImpl do not need to take cellLock in visitOutputConstraints and ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-07-27 02:40 PDT by Yusuke Suzuki
Modified: 2022-02-24 00:58 PST (History)
7 users (show)

See Also:

Attachments
Patch (4.06 KB, patch)
2019-07-27 02:43 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (4.39 KB, patch)
2022-02-23 17:33 PST, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (4.75 KB, patch)
2022-02-23 17:36 PST, Yusuke Suzuki 		mark.lam: review+
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2019-07-27 02:40:35 PDT 
[JSC] WeakMapImpl do not need to take cellLock in visitOutputConstraint and main thread
Comment 1 Yusuke Suzuki 2019-07-27 02:43:22 PDT 
Created attachment 375026 [details]
Patch
Comment 2 Yusuke Suzuki 2022-02-23 17:33:03 PST 
Created attachment 453059 [details]
Patch
Comment 3 Yusuke Suzuki 2022-02-23 17:36:35 PST 
Created attachment 453060 [details]
Patch
Comment 4 Mark Lam 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.
Comment 5 Yusuke Suzuki 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.
Comment 6 Yusuke Suzuki 2022-02-24 00:57:20 PST 
Committed r290416 (247724@trunk): <https://commits.webkit.org/247724@trunk>
Comment 7 Radar WebKit Bug Importer 2022-02-24 00:58:20 PST 
<rdar://problem/89405434>