Closed
Bug 629601
Opened 14 years ago
Closed 11 years ago
JM: shrink ValueRemat
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: n.nethercote, Unassigned)
References
Details
(Whiteboard: [MemShrink:P3])
Attachments
(1 file)
6.73 KB,
patch
|
Details | Diff | Splinter Review |
ValueRemat looks like this: struct ValueRemat { union { struct { union { int32 typeRemat_; JSValueType knownType_; } type; int32 dataRemat_ : MIN_STATE_REMAT_BITS; bool isTypeKnown_ : 1; } s; jsval v_; } u; bool isConstant_ : 1; bool isDataSynced : 1; bool isTypeSynced : 1; ... various methods here ... } sizeof(ValueRemat) is 16 bytes, on both 32-bit and 64-bit platforms. This is annoying, because it's really 8 bytes (for a jsval) plus 3 bits. Furthermore, if ValueRemat shrinks, PICInfo also shrinks, which is a win (see bug 615199 comment 91). Can we stuff those 3 bits into the NaN space in the jsval? We'd have to do it in a way that works when 's' is being used instead of 'v_'.
Comment 1•14 years ago
|
||
(In reply to comment #0) > Can we stuff those 3 bits into the NaN space in the jsval? We'd have to do it > in a way that works when 's' is being used instead of 'v_'. Cool idea! IIRC we set 16 of the high bits instead of the necessary 11, so we can burn three with one bit to distinguish from the system's canonical NaN? (I forget what luke said was the pattern for the system canonical NaNs.) If v_.isDouble() you know it has to be a double constant, in which case we don't have to worry about data/type sync'd.
Those extra two bits aren't really part of the remat info. They're only used during compilation. They should be taken out and put into a new struct that wraps ValueRemat (maybe called ValueRematState or something). With only one bit the problem is a little simpler. The type can be encoded in 6 bits (5 for reg/const, 1 for differentiating). Data needs 21 bits, so 27 bits total for the non-constant case. For the constant case, the jsval can be as normal, and for the non-constant case, the jsval can be JSVAL_TYPE_MAGIC with the remat info squirreled into the payload. We'd just need to reserve a portion of that "why" space in jsval.h, because IIRC we can place JSVAL_MAGIC_HOLE in there. Still, I think there's more to gain by subclassing PICInfo into SetPropIC or something. There's way more non-SET ICs than there are SETs, and that'd get rid up to 12-bytes per, if you pack typeCheckOffset a little better.
Comment 3•14 years ago
|
||
(In reply to comment #2) Ah yes, using an impossible-constant type as the discriminant makes way more sense. The idea of more NaN-tomfoolery alluded to in the OP sent me down a weird path. :-)
Reporter | ||
Comment 4•13 years ago
|
||
(In reply to comment #2) > > Still, I think there's more to gain by subclassing PICInfo into SetPropIC or > something. There's way more non-SET ICs than there are SETs, and that'd get rid > up to 12-bytes per, if you pack typeCheckOffset a little better. I spun this idea off as bug 630738.
Reporter | ||
Updated•13 years ago
|
Blocks: mslim-fx5+
Reporter | ||
Updated•13 years ago
|
Whiteboard: [MemShrink:P3]
Updated•13 years ago
|
Assignee: general → pbiggar
Comment 5•13 years ago
|
||
First two bits were pretty easy anyway.
Updated•13 years ago
|
Assignee: paul.biggar → general
Comment 6•11 years ago
|
||
JM was removed.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•