Bug 162142 – [JSC] Use is_cell_with_type for @isRegExpObject, @isMap, and @isSet

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Yusuke Suzuki]

This may offer performance benefit!

[Yusuke Suzuki]

Created attachment 289242 [details]
Patch

[Yusuke Suzuki]

# Baseline
Air
firstIteration:     73.81 ms +- 0.92 ms
averageWorstCase:   41.55 ms +- 0.78 ms
steadyState:        3492.46 ms +- 14.59 ms
summary:            157.36 ms +- 1.30 ms

Basic
firstIteration:     35.94 ms +- 0.42 ms
averageWorstCase:   20.46 ms +- 0.82 ms
steadyState:        1966.58 ms +- 18.93 ms
summary:            157.32 ms +- 1.29 ms

# Patched
Air
firstIteration:     74.32 ms +- 1.70 ms
averageWorstCase:   41.54 ms +- 0.77 ms
steadyState:        3451.03 ms +- 16.79 ms
summary:            156.78 ms +- 1.32 ms

Basic
firstIteration:     36.60 ms +- 0.56 ms
averageWorstCase:   20.03 ms +- 0.76 ms
steadyState:        1940.35 ms +- 15.14 ms
summary:            156.71 ms +- 1.32 ms

[Michael Saboff]

Comment on attachment 289242 [details]
Patch

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

r=me.

> Source/JavaScriptCore/ChangeLog:10
> +        Previously, they are implemented as functions and only @isRegExpObject
> +        is handled in DFG and FTL.

Use past tense throughout the sentence.  I suggest "Previously, they were implemented as functions and only @isRegExpObject was handled in the DFG and FTL."

> Source/JavaScriptCore/ChangeLog:12
> +        Recent op_is_cell_with_type and DFG IsCellWithType node allow us
> +        to implement the above ones in all the JIT tiers easily.

Awkward wording.  Suggest - "The recently added op_is_cell_with_type bytecode and DFG IsCellWithType node allows us to simplify the above checks in all JIT tiers."

> Source/JavaScriptCore/ChangeLog:13
> +        We change above ones to bytecode intrinsic and emit op_is_cell_with_type.

I suggest changing this sentence to "Changed these checks to bytecode intrinsics using op_is_cell_with type."

[Yusuke Suzuki]

Comment on attachment 289242 [details]
Patch

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

Thank you so much!

>> Source/JavaScriptCore/ChangeLog:10
>> +        is handled in DFG and FTL.
> 
> Use past tense throughout the sentence.  I suggest "Previously, they were implemented as functions and only @isRegExpObject was handled in the DFG and FTL."

Fixed.

>> Source/JavaScriptCore/ChangeLog:12
>> +        to implement the above ones in all the JIT tiers easily.
> 
> Awkward wording.  Suggest - "The recently added op_is_cell_with_type bytecode and DFG IsCellWithType node allows us to simplify the above checks in all JIT tiers."

Fixed.

>> Source/JavaScriptCore/ChangeLog:13
>> +        We change above ones to bytecode intrinsic and emit op_is_cell_with_type.
> 
> I suggest changing this sentence to "Changed these checks to bytecode intrinsics using op_is_cell_with type."

Fixed.

[Yusuke Suzuki]

Committed r206104: <http://trac.webkit.org/changeset/206104>