Last Comment Bug 629601 - JM: shrink ValueRemat
: JM: shrink ValueRemat
Status: RESOLVED WONTFIX
[MemShrink:P3]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal with 2 votes (vote)
: ---
Assigned To: general
:
Mentors:
Depends on:
Blocks: JaegerShrink mslim-fx5+
  Show dependency treegraph
 
Reported: 2011-01-27 21:57 PST by Nicholas Nethercote [:njn] (on vacation until July 11)
Modified: 2013-06-10 09:54 PDT (History)
19 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
WIP: first 2 bits (6.73 KB, patch)
2011-06-16 17:03 PDT, Paul Biggar
no flags Details | Diff | Review

Description Nicholas Nethercote [:njn] (on vacation until July 11) 2011-01-27 21:57:50 PST
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 Chris Leary [:cdleary] (not checking bugmail) 2011-01-28 11:21:39 PST
(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.
Comment 2 David Anderson [:dvander] 2011-01-28 11:33:52 PST
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 Chris Leary [:cdleary] (not checking bugmail) 2011-01-28 11:58:28 PST
(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. :-)
Comment 4 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-02-01 17:31:15 PST
(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.
Comment 5 Paul Biggar 2011-06-16 17:03:35 PDT
Created attachment 539943 [details] [diff] [review]
WIP: first 2 bits

First two bits were pretty easy anyway.
Comment 6 Jan de Mooij [:jandem] 2013-06-10 09:54:34 PDT
JM was removed.

Note You need to log in before you can comment on or make changes to this bug.