Closed Bug 1318815 Opened 8 years ago Closed 8 years ago

Rethink about ClassSpec delegation based on current structure.

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
minor

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: arai, Assigned: arai)

References

Details

Attachments

(1 file)

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.
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.
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: nobody → arai.unmht
Status: NEW → ASSIGNED
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 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+
https://hg.mozilla.org/integration/mozilla-inbound/rev/177b7924440c450525d1127c9c2f1d3d642e4a2c Bug 1318815 - Remove ClassSpec delegation and point target ClassSpec directly from prototype class. r=jandem
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.

Attachment

General

Created:
Updated:
Size: