[JSC] Introduce WriteBarrierStructureID

• Component: New Bugs
• Status: RESOLVED FIXED

[Yusuke Suzuki]

[JSC] Introduce WriteBarrierStructureID

[Yusuke Suzuki]

Created attachment 446178 [details]
Patch

[Yusuke Suzuki]

Created attachment 446259 [details]
Patch

[Yusuke Suzuki]

Created attachment 446262 [details]
Patch

[Yusuke Suzuki]

Created attachment 446270 [details]
Patch

[Yusuke Suzuki]

Created attachment 446273 [details]
Patch

[Mark Lam]

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.

[Yusuke Suzuki]

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.

[Yusuke Suzuki]

Committed r286667 (244975@trunk): <https://commits.webkit.org/244975@trunk>

[Radar WebKit Bug Importer]

<rdar://problem/86220662>