Closed
Bug 1318815
Opened 8 years ago
Closed 8 years ago
Rethink about ClassSpec delegation based on current structure.
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla53
Tracking | Status | |
---|---|---|
firefox53 | --- | fixed |
People
(Reporter: arai, Assigned: arai)
References
Details
Attachments
(1 file)
(deleted),
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•8 years ago
|
||
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.
Assignee | ||
Comment 2•8 years ago
|
||
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.
Assignee | ||
Comment 3•8 years ago
|
||
try run that removes delegation
https://treeherder.mozilla.org/#/jobs?repo=try&revision=30f7854959afe4b049d61a24c43abc9c478d7735
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•8 years ago
|
||
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
Attachment #8812645 -
Flags: review?(jdemooij)
Comment 5•8 years ago
|
||
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!
Attachment #8812645 -
Flags: review?(jdemooij) → review+
Assignee | ||
Comment 6•8 years ago
|
||
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•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in
before you can comment on or make changes to this bug.
Description
•