553824 - Squeeze more chars into the short string using JSString::mOffset

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Igor Bukanov]

+++ This bug was initially created as a clone of Bug #553541 +++

The short strings from the bug 553541 can hold more chars if JSString::mOffset would be made the last field in JSString and shared with short string chars.

Comment 1

[Andreas Gal :gal]

I did some quick sampling of the string population of gmail and SS. I have now serious doubts this patch will do anything but complicate the code (the overlap is really weird). I would like to see real world or SS speedups before we land a fix for this bug.

Comment 2

[Igor Bukanov]

(In reply to comment #1)
> but complicate the code (the overlap is really weird).

Hm, what kind of code of code complexity this brings? AFAICS the necessary changes would be to replace mOffset with union { size_t mOffset; jschar data[1]; }, add a static assert to verify that the offset of data matches mOffset, and add a const to denote the real string size. With that there would be no need for JSShortString.

Comment 3

[Alan Pierce]

The fix for 578205 (which redid the backed-out patch for 553541) keeps the first few string characters in the first string header in JSShortString, which essentially fixes this issue.