1318815 - Rethink about ClassSpec delegation based on current structure.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Tooru Fujisawa [:arai]]

At the point of ClassSpec delegation was introduced (bug 861219), ClassSpec was embedded into Class,
and I think that was one of the main reason why delegation was introduced.

now ClassSpec is separated from Class (bug 1260984), and we can just point other ClassSpec, perhaps a ClassSpec from multiple Class'es.
so, in some case, we could remove delegation, depending on `flags` member.

Comment 1

[Tooru Fujisawa [:arai]]

If ClassSpec is delegated the following method just returns delegated ClassSpec's one:
  * createConstructorHook (for createConstructor_)
  * createPrototypeHook (for createPrototype_)
  * constructorFunctions (for constructorFunctions_)
  * constructorProperties (for constructorProperties_)
  * prototypeFunctions (for prototypeFunctions_)
  * prototypeProperties (for prototypeProperties_)
  * finishInitHook (for finishInit_)

and the following methods are used only internally:
  * defined
    always true if delegated() is true,
    also true for all existing delegated ClassSpecs
      * PromiseObjectClassSpec
      * DateObjectClassSpec
      * ArrayBufferObjectClassSpec
      * TypedArrayObjectClassSpecs
  * delegated
  * delegatedClassSpec

The only difference are the following 2 methods, that behaves differently depending on |flags| field
  * inheritanceProtoKey
    depends |flags & ProtoKeyMask|
  * shouldDefineConstructor
    depends |flags & DontDefineConstructor|


If above flags are same between delegation source and target, we can just point delegated ClassSpec instead of using IsDelegated.

Here's current consumers:
  * Promise
      source:       PromiseObjectProtoClassSpec
      source.flags: ClassSpec::IsDelegated
      target:       PromiseObjectClassSpec
      target.flags: 0

  * Date
      source:       DateObjectProtoClassSpec
      source.flags: ClassSpec::IsDelegated
      target:       DateObjectClassSpec
      target.flags: 0

  * ArrayBuffer
      source:       ArrayBufferObjectProtoClassSpec
      source.flags: ClassSpec::IsDelegated
      target:       ArrayBufferObjectClassSpec
      target.flags: 0

  * SharedArrayBuffer
      source:       SharedArrayBufferObjectProtoClassSpec
      source.flags: ClassSpec::IsDelegated
      target:       ArrayBufferObjectClassSpec
      target.flags: 0

  * %TypedArray%
      source:       TypedArrayObjectProtoClassSpecs
      source.flags: JSProto_TypedArray | ClassSpec::IsDelegated
      target:       TypedArrayObjectClassSpecs
      target.flags: JSProto_TypedArray

|flags & ProtoKeyMask| and |flags & DontDefineConstructor| are same in all cases, and apparently we don't need delegation for current usage.

Comment 2

[Tooru Fujisawa [:arai]]

I forgot 2 new consumers

  * Map
      source:       MapObjectProtoClassSpec
      source.flags: ClassSpec::IsDelegated
      target:       MapObject::classSpec_
      target.flags: 0

  * Set
      source:       SetObjectProtoClassSpec
      source.flags: ClassSpec::IsDelegated
      target:       SetObject::classSpec_
      target.flags: 0

so they're also okay.

Comment 3

[Tooru Fujisawa [:arai]]

try run that removes delegation
https://treeherder.mozilla.org/#/jobs?repo=try&revision=30f7854959afe4b049d61a24c43abc9c478d7735

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: Remove ClassSpec delegation and point target ClassSpec directly from prototype class.

Partially backed out bug 861219 patch that introduces delegation,
so now there's no delegation.

and also changed the reference to ClassSpec with ClassSpec::IsDelegated to the reference to delegation target.
and moved prototype class to class static member to avoid forward declaration in cpp files.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=30f7854959afe4b049d61a24c43abc9c478d7735

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 8812645 [details] [diff] [review]
Remove ClassSpec delegation and point target ClassSpec directly from prototype class.

Review of attachment 8812645 [details] [diff] [review]:
-----------------------------------------------------------------

This is a great refactoring and it really simplifies things. Nice work!

Comment 6

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/177b7924440c450525d1127c9c2f1d3d642e4a2c
Bug 1318815 - Remove ClassSpec delegation and point target ClassSpec directly from prototype class. r=jandem

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/177b7924440c