Bug 233918 - [JSC] Introduce WriteBarrierStructureID
Summary: [JSC] Introduce WriteBarrierStructureID
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: 233959
  Show dependency tree / graph
 
Reported: 2021-12-07 01:39 PST by Yusuke Suzuki
Modified: 2021-12-08 11:12 PST (History)
8 users (show)

See Also:

Attachments
Patch (35.57 KB, patch)
2021-12-07 08:52 PST, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (55.74 KB, patch)
2021-12-07 17:11 PST, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (61.17 KB, patch)
2021-12-07 17:28 PST, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (62.89 KB, patch)
2021-12-07 17:42 PST, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (73.74 KB, patch)
2021-12-07 18:20 PST, Yusuke Suzuki 		mark.lam: review+
Details | Formatted Diff | Diff
Show Obsolete (4) 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 2021-12-07 01:39:29 PST 
[JSC] Introduce WriteBarrierStructureID
Comment 1 Yusuke Suzuki 2021-12-07 08:52:18 PST 
Created attachment 446178 [details]
Patch
Comment 2 Yusuke Suzuki 2021-12-07 17:11:49 PST 
Created attachment 446259 [details]
Patch
Comment 3 Yusuke Suzuki 2021-12-07 17:28:10 PST 
Created attachment 446262 [details]
Patch
Comment 4 Yusuke Suzuki 2021-12-07 17:42:59 PST 
Created attachment 446270 [details]
Patch
Comment 5 Yusuke Suzuki 2021-12-07 18:20:35 PST 
Created attachment 446273 [details]
Patch
Comment 6 Mark Lam 2021-12-08 01:08:41 PST 
Comment on attachment 446273 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=446273&action=review

r=me

> Source/JavaScriptCore/runtime/WriteBarrier.h:278
> +    // Should only be used by JSCell during early initialisation
> +    // when some basic types aren't yet completely instantiated

Why this comment?  In this patch, `setEarlyValue` looks more like a `setInternal` and can be made private.  Is this needed in code that is yet to come?

> Source/JavaScriptCore/runtime/WriteBarrier.h:283
> +        // Copy m_cell to a local to avoid multiple-read issues. (See <http://webkit.org/b/110854>)

/m_cell/m_structureID/

> Source/JavaScriptCore/runtime/WriteBarrier.h:313
> +        m_structureID = StructureID { };

Do we need the `StructureID` here?  Wouldn't it be implied by the type of m_structureID?

> Source/JavaScriptCore/runtime/WriteBarrier.h:332
> +            m_structureID = StructureID { };

Ditto.

> Source/JavaScriptCore/runtime/WriteBarrierInlines.h:82
> +        m_structureID = StructureID { };

Ditto.
Comment 7 Yusuke Suzuki 2021-12-08 11:04:58 PST 
Comment on attachment 446273 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=446273&action=review

>> Source/JavaScriptCore/runtime/WriteBarrier.h:278
>> +    // when some basic types aren't yet completely instantiated
> 
> Why this comment?  In this patch, `setEarlyValue` looks more like a `setInternal` and can be made private.  Is this needed in code that is yet to come?

Because it is aligned to WriteBarrier<JSCell>, so same methods and same comments are prepared. So, we should use setEarlyValue to align it to WriteBarrier<>.

>> Source/JavaScriptCore/runtime/WriteBarrier.h:283
>> +        // Copy m_cell to a local to avoid multiple-read issues. (See <http://webkit.org/b/110854>)
> 
> /m_cell/m_structureID/

Changed.

>> Source/JavaScriptCore/runtime/WriteBarrier.h:313
>> +        m_structureID = StructureID { };
> 
> Do we need the `StructureID` here?  Wouldn't it be implied by the type of m_structureID?

Dropped.

>> Source/JavaScriptCore/runtime/WriteBarrier.h:332
>> +            m_structureID = StructureID { };
> 
> Ditto.

Ditto.

>> Source/JavaScriptCore/runtime/WriteBarrierInlines.h:82
>> +        m_structureID = StructureID { };
> 
> Ditto.

Ditto.
Comment 8 Yusuke Suzuki 2021-12-08 11:11:57 PST 
Committed r286667 (244975@trunk): <https://commits.webkit.org/244975@trunk>
Comment 9 Radar WebKit Bug Importer 2021-12-08 11:12:22 PST 
<rdar://problem/86220662>