Closed
Bug 588774
Opened 14 years ago
Closed 11 years ago
Make subclasses of JSObject
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: jorendorff, Unassigned)
Details
The interface of JSObject is absurdly fat. For example, it has 11 methods and 5 constants that are only meaningful for Arguments objects; and more for Date, Function, RegExp, and so on. Even XML. Inline member functions of JSObject are defined in about 11 different header files now. This is outrageously silly. Those should be subclasses.
Comment 1•14 years ago
|
||
It's true. Nick did help encapsulate raw slots usage (bug 565845 still lives but he resolved a bunch, it was good work), which alas made JSObject a lot wider. It was a calculated trade-off and not the final word. We talked then about getting to the subtypes promised land. First fatten JSObject, then split it up. Waldo raised the desire for typed members instead of tagged fslots in bug 429507 comment 15. We all feel it. There's some tension with the variable fslots based on a allocation-site cache prediction work that is imminent after bug 558451 is landed: bhackett owns that in bug 584917 (see also earlier bug 547327). It seems to me we could have subtypes and variable length fixed slots, but the latter would have to come after members added by the former, or else we'd have to pin the number of fixed slots per subtype. The variable length fslots wins are mainly important for Object and Array (possibly Function too, needs study), so the conflict may be less than it could be, in practice. /be
Comment 2•14 years ago
|
||
Is the desire for a better interface, or subtypes that extend the data fields of JSObject? Both should be compatible with 584917, but the former is way easier --- just make subtypes of JSObject which define new member functions but no new fields. I'm not sure what the latter is buying; most objects (except dates) have few reserved slots that hold non-values, so this would sometimes save a few bytes while probably increasing complexity in both the GC and 584917 quite a bit.
Reporter | ||
Comment 3•14 years ago
|
||
(In reply to comment #2) > Is the desire for a better interface, or subtypes that extend the data fields > of JSObject? Better interface is plenty for one bug.
Comment 4•14 years ago
|
||
Is the basic plan still to have a pair bool JSObject::isX() X *JSObject::asX() for X = { RegExp, Args, DenseArray, ... } ?
Reporter | ||
Comment 5•14 years ago
|
||
Comment 4 would be fine with me. We could use templates instead. template <class T> X *is() { return clasp == T::classSingleton(); } template <class T> X *to() { JS_ASSERT(is<T>()); return (T *) this; } template <class T> X *as() { return is<T>() ? to<T>() : NULL; } but maybe obj->is<FunctionObject>() is just too ugly compared to obj->isFunction(). This could be taken a little further, so that: if (FunctionObject *funobj = obj->dcast()) ... if (FunctionObject *funobj = value.dcast()) ... a la do_QueryInterface. template<class T> operator T*() seems to work in MSVC. Not that that makes it a good idea.
Comment 6•14 years ago
|
||
(In reply to comment #0) > > This is outrageously silly. Those should be subclasses. I'm glad you noticed. See bug 566789, which starts fixing this. Its patch has r+ from Waldo, unofficial r+ from Luke, and (IIRC) tacit approval from Brendan. The sub-classes don't have extra data fields. I also have numerous other bugs open (eg. bug 565845) to fully encapsulate fslots and dslots (several which also have r+) which would make things like type-specific slots simpler to implement. But all this mere clean-up work has been languishing while big perf-critical bugs like fatvals and Brendan's scope removal have been in progress.
Comment 7•13 years ago
|
||
Is this bug still active / valid?
Comment 8•13 years ago
|
||
Yes. I've been carrying a patch to add NumberObject for months now (waiting on other patches in my mq to land). Others will come over time. It's not a huge priority, tho, so it only happens as people make the effort.
Comment 10•12 years ago
|
||
RegExp has its own subclass now; ditto for Boolean, Number, and String. I don't believe Date does yet, but I could be mistaken about that. Call objects have a hierarchy of subclasses deriving from the newly-named ScopeObject class. XML objects don't, and I suspect probably won't given the low value and moderate cost to making it happen there. Basically, this has been happening piecemeal in other bugs for quite some time. It's probably not necessary to have a bug specifically devoted to it. If this were my bug I'd just close it and let the separate-bug approach keep doing its thing, but that's enough a matter of personal hygiene I'm not going to do it to a bug that's not mine, that I didn't file.
Comment 11•12 years ago
|
||
jorendorff, would you respond to comment 10 and/or take action?
Comment 12•12 years ago
|
||
This seems to be a dead bug.. jorendorff?
Updated•11 years ago
|
Flags: needinfo?(jorendorff)
Reporter | ||
Comment 13•11 years ago
|
||
==> RESO WORKSFORME.
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: needinfo?(jorendorff)
Resolution: --- → WORKSFORME
Comment 14•11 years ago
|
||
FWIW, I've just about finished converting all of these. See bug 884124 and bug 887558.
You need to log in
before you can comment on or make changes to this bug.
Description
•