Closed Bug 629601 Opened 14 years ago Closed 11 years ago

JM: shrink ValueRemat

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: n.nethercote, Unassigned)

References

Details

(Whiteboard: [MemShrink:P3])

Attachments

(1 file)

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_'.
(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.
(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. :-)
(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.
Blocks: mslim-fx5+
Whiteboard: [MemShrink:P3]
Assignee: general → pbiggar
First two bits were pretty easy anyway.
Assignee: paul.biggar → general
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.

Attachment

General

Creator:
Created:
Updated:
Size: