Closed
Bug 630738
Opened 13 years ago
Closed 13 years ago
JM: distinguish "get", "set" and "other" PICs to save space
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: n.nethercote, Assigned: n.nethercote)
References
Details
Attachments
(1 file)
60.20 KB,
patch
|
Details | Diff | Splinter Review |
Spun off from bug 629601 comment 2: "get" PICs have some specific info, "set" PICs have some specific info, "other" ("bind" and "name") PICs have no specific info. If we sub-type them we can save some space in "get" and "other" PICs: - 32-bit: 64 B --> 56/64/48 B - 64-bit: 112 B --> 104/112/96 B ie. the size of "set" PICs will be unchanged, "get" PICs (the most common) will shrink by 8 bytes, "other" PICS will shrink by 16 bytes. Tracking three kinds of PICs instead of one in JITScript will cost an extra 8 bytes once bug 630445 is done. Because PICs are much more common than JITScripts, this will be a net win.
Assignee | ||
Updated•13 years ago
|
Summary: JM: distinguish Set PICs to save space → JM: distinguish "get", "set" and "other" PICs to save space
Assignee | ||
Comment 1•13 years ago
|
||
Also, if 26 bits is enough for typeCheckOffset, it could be compacted into the same int32 as typeReg and hasTypeCheck, saving another 4 bytes for "get" PICs.
26 bits should be more than enough. The get/callprop ICs have more than one exit block in their slow path, and we used to sync a huge amount of stuff in there. Now that offset can probably be packed in 12 bits. Just add an assert in Compiler.cpp that after setting it, its value is equal to the distance we expect. Doing that locally, packing in 12-bits, and running jit-tests with --jitflags=m,md, I get no failures.
Assignee | ||
Comment 3•13 years ago
|
||
After reducing typeCheckOffset to 26 bits, I get these get/set/other sizes: 32-bit: 52/64/48 bytes 64-bit: 96/112/96 bytes Note that "get" and "other" size for 64-bit are the same. Nice. Maybe just distinguishing "set" and "non-set" might be enough. I'll have to measure how common "other" ones are.
Assignee | ||
Updated•13 years ago
|
Blocks: JaegerShrink
Assignee | ||
Comment 4•13 years ago
|
||
I had this working, then some changes occurred and I updated and merged and I get this assertion a lot: Assertion failure: getPic->hasTypeCheck(), at ../methodjit/PolyIC.cpp:888 Not sure what the problem is. Furthermore, now that bug 631951 has landed, the memory reduction gained by this patch will be roughly 3--4x smaller, and so may not be worth bothering with.
Assignee | ||
Updated•13 years ago
|
Blocks: mslim-fx5+
Assignee | ||
Comment 5•13 years ago
|
||
Doesn't seem worth it now, see comment 4.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•