Bug 195498 – [JSC] Reduce # of structures in JSGlobalObject initialization

• Component: New Bugs
• Status: RESOLVED FIXED

[Yusuke Suzuki]

[JSC] Reduce # of structures in JSGlobalObject initialization

[Yusuke Suzuki]

Created attachment 364095 [details]
Patch

[Yusuke Suzuki]

Created attachment 364096 [details]
Patch

[Yusuke Suzuki]

Created attachment 364106 [details]
Patch

[Darin Adler]

Comment on attachment 364106 [details]
Patch

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

I presume there is enough regression test coverage of these things that we have some good confidence we got them all right.

> Source/JavaScriptCore/runtime/ArrayPrototype.cpp:130
> +    const Identifier unscopableNames[] = {

There is some reference count churn initializing an array of Identifier. Might be nice to look for an idiom with less wasted work. For example, it would be slightly more efficient to make an array of const Identifier* and then dereference all of them to avoid 9 ref/deref pairs.

> Source/JavaScriptCore/runtime/VM.h:572
> +    Structure* setIteratorStructure()
> +    {
> +        if (LIKELY(m_setIteratorStructure))
> +            return m_setIteratorStructure.get();
> +        return setIteratorStructureSlow();
> +    }

I prefer an idiom where we don’t have so many multiline function bodies in a class definition. For me they interfere with my ability to use the class definition to get an overview of what’s in the class. The pattern is also easy to avoid by moving inlines function bodies down in the header, outside the class definition, but with the cost of not being able to see immediately what each function does by seeing its implementation. But it seems that VM already has tons of these. So I don’t think more are going to make things worse.

[Yusuke Suzuki]

Comment on attachment 364106 [details]
Patch

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

>> Source/JavaScriptCore/runtime/ArrayPrototype.cpp:130
>> +    const Identifier unscopableNames[] = {
> 
> There is some reference count churn initializing an array of Identifier. Might be nice to look for an idiom with less wasted work. For example, it would be slightly more efficient to make an array of const Identifier* and then dereference all of them to avoid 9 ref/deref pairs.

`const Identifier*` sounds better. Fixed.

>> Source/JavaScriptCore/runtime/VM.h:572
>> +    }
> 
> I prefer an idiom where we don’t have so many multiline function bodies in a class definition. For me they interfere with my ability to use the class definition to get an overview of what’s in the class. The pattern is also easy to avoid by moving inlines function bodies down in the header, outside the class definition, but with the cost of not being able to see immediately what each function does by seeing its implementation. But it seems that VM already has tons of these. So I don’t think more are going to make things worse.

I leave it as is since (1) as you said, VM already has bunch of inlined functions in class declaration, and (2) it is aligned to the existing lazy allocation methods in VM.
I think we can clean up VM at some point.

[Yusuke Suzuki]

Committed r242742: <https://trac.webkit.org/changeset/242742>

[Radar WebKit Bug Importer]

<rdar://problem/48784197>