1490985 - Add the same string and static string optimizations from NewDependentString to CreateDependentString

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement)

Description

[André Bargull [:anba]]

This [1] and a simplified version of this one [2] can easily be added to CreateDependentString. 

[1] https://searchfox.org/mozilla-central/rev/a23c3959b62cd3f616435e02810988ef5bac9031/js/src/vm/StringType.cpp#1516-1518
[2] https://searchfox.org/mozilla-central/rev/a23c3959b62cd3f616435e02810988ef5bac9031/js/src/vm/StringType.cpp#1528-1531


Using an instrumented browser showed that adding these optimizations can help to reduce string allocations for RegExp matching. I've visited the following sites and clicked a bit around to gather these results:

spiegel.de
empty (Latin-1):      2.4% (3046)
empty (Two-Byte):     0% (0)
all (Latin-1):        6.1% (7778)
all (Two-Byte):       1.8% (2360)
static (Latin-1):     25.4% (31945)
thin (Latin-1):       56.2% (70592)
thin (Two-Byte):      2.5% (3205)
fat (Latin-1):        0.9% (1195)
fat (Two-Byte):       0.7% (952)
dependent (Latin-1):  1.2% (1631)
dependent (Two-Byte): 2.2% (2779)
Summary: 33.3% (42083 of 125483) allocation can be saved

youtube.com
empty (Latin-1):      0% (0)
empty (Two-Byte):     0% (0)
all (Latin-1):        10.6% (1244)
all (Two-Byte):       0.4% (51)
static (Latin-1):     32.4% (3776)
thin (Latin-1):       52.8% (6154)
thin (Two-Byte):      0.4% (48)
fat (Latin-1):        0.6% (74)
fat (Two-Byte):       0% (3)
dependent (Latin-1):  1.8% (211)
dependent (Two-Byte): 0.6% (79)
Summary: 43.4% (5071 of 11640) allocation can be saved

twitch.tv
empty (Latin-1):      44.1% (89045)
empty (Two-Byte):     0% (53)
all (Latin-1):        0.4% (894)
all (Two-Byte):       0.1% (314)
static (Latin-1):     11.1% (22564)
thin (Latin-1):       38.9% (78597)
thin (Two-Byte):      4.8% (9733)
fat (Latin-1):        0% (100)
fat (Two-Byte):       0% (54)
dependent (Latin-1):  0% (2)
dependent (Two-Byte): 0.2% (536)
Summary: 11.6% (23772 of 201892) allocation can be saved

facebook.com
empty (Latin-1):      0.5% (894)
empty (Two-Byte):     0% (1)
all (Latin-1):        6.1% (9269)
all (Two-Byte):       1% (1538)
static (Latin-1):     7.7% (11692)
thin (Latin-1):       76.6% (115495)
thin (Two-Byte):      1.9% (2928)
fat (Latin-1):        1.5% (2409)
fat (Two-Byte):       0% (1)
dependent (Latin-1):  1.2% (1818)
dependent (Two-Byte): 3% (4601)
Summary: 14.8% (22499 of 150646) allocation can be saved

instagram.com
empty (Latin-1):      3.2% (191)
empty (Two-Byte):     2.4% (143)
all (Latin-1):        7.9% (461)
all (Two-Byte):       0.3% (22)
static (Latin-1):     20.2% (1175)
thin (Latin-1):       56.9% (3301)
thin (Two-Byte):      3.7% (217)
fat (Latin-1):        0.8% (49)
fat (Two-Byte):       1.7% (100)
dependent (Latin-1):  0% (1)
dependent (Two-Byte): 2.3% (137)
Summary: 28.4% (1658 of 5797) allocation can be saved

wikipedia.org
empty (Latin-1):      0% (0)
empty (Two-Byte):     0% (0)
all (Latin-1):        21.6% (5394)
all (Two-Byte):       0.3% (82)
static (Latin-1):     27.6% (6882)
thin (Latin-1):       45.1% (11259)
thin (Two-Byte):      0.7% (198)
fat (Latin-1):        0.9% (235)
fat (Two-Byte):       0.5% (135)
dependent (Latin-1):  1.1% (277)
dependent (Two-Byte): 1.8% (461)
Summary: 49.5% (12358 of 24923) allocation can be saved

twitter.com
empty (Latin-1):      0.1% (81)
empty (Two-Byte):     0.1% (123)
all (Latin-1):        35.2% (27704)
all (Two-Byte):       0.1% (153)
static (Latin-1):     9.9% (7847)
thin (Latin-1):       15.1% (11910)
thin (Two-Byte):      4.7% (3765)
fat (Latin-1):        1.4% (1103)
fat (Two-Byte):       3.9% (3096)
dependent (Latin-1):  1.1% (927)
dependent (Two-Byte): 27.8% (21843)
Summary: 45.2% (35704 of 78552) allocation can be saved

amazon.it
empty (Latin-1):      6.7% (6561)
empty (Two-Byte):     0% (0)
all (Latin-1):        17% (16588)
all (Two-Byte):       0% (20)
static (Latin-1):     50.1% (48708)
thin (Latin-1):       19.1% (18630)
thin (Two-Byte):      0.5% (574)
fat (Latin-1):        3.5% (3418)
fat (Two-Byte):       0% (96)
dependent (Latin-1):  2.3% (2271)
dependent (Two-Byte): 0.3% (338)
Summary: 67.1% (65316 of 97204) allocation can be saved

google.com
empty (Latin-1):      0.1% (4)
empty (Two-Byte):     0% (0)
all (Latin-1):        37.1% (811)
all (Two-Byte):       0% (1)
static (Latin-1):     56.3% (1231)
thin (Latin-1):       6.2% (136)
thin (Two-Byte):      0% (1)
fat (Latin-1):        0% (0)
fat (Two-Byte):       0% (0)
dependent (Latin-1):  0% (0)
dependent (Two-Byte): 0% (1)
Summary: 93.4% (2043 of 2185) allocation can be saved

yahoo.com
empty (Latin-1):      0.2% (3)
empty (Two-Byte):     0% (0)
all (Latin-1):        0.9% (14)
all (Two-Byte):       0.9% (14)
static (Latin-1):     0% (0)
thin (Latin-1):       2.3% (35)
thin (Two-Byte):      74.3% (1093)
fat (Latin-1):        0.3% (5)
fat (Two-Byte):       4.2% (63)
dependent (Latin-1):  0% (0)
dependent (Two-Byte): 16.5% (244)
Summary: 1.8% (28 of 1471) allocation can be saved

reddit.com
empty (Latin-1):      0% (0)
empty (Two-Byte):     0% (0)
all (Latin-1):        53.3% (11703)
all (Two-Byte):       0% (3)
static (Latin-1):     14.9% (3275)
thin (Latin-1):       30.9% (6798)
thin (Two-Byte):      0.1% (38)
fat (Latin-1):        0% (11)
fat (Two-Byte):       0% (0)
dependent (Latin-1):  0.4% (104)
dependent (Two-Byte): 0% (6)
Summary: 68.2% (14981 of 21938) allocation can be saved

imdb.com
empty (Latin-1):      0.2% (129)
empty (Two-Byte):     0% (0)
all (Latin-1):        41% (20860)
all (Two-Byte):       0% (0)
static (Latin-1):     21.5% (10961)
thin (Latin-1):       34.8% (17695)
thin (Two-Byte):      0% (0)
fat (Latin-1):        1.6% (859)
fat (Two-Byte):       0% (0)
dependent (Latin-1):  0.6% (325)
dependent (Two-Byte): 0% (0)
Summary: 62.5% (31821 of 50829) allocation can be saved

docs.google.com
empty (Latin-1):      0.6% (3021)
empty (Two-Byte):     0% (5)
all (Latin-1):        18.4% (83231)
all (Two-Byte):       0% (378)
static (Latin-1):     1.9% (8674)
thin (Latin-1):       35.9% (162407)
thin (Two-Byte):      23.1% (104525)
fat (Latin-1):        10.4% (47439)
fat (Two-Byte):       1.2% (5553)
dependent (Latin-1):  7.8% (35659)
dependent (Two-Byte): 0.2% (1317)
Summary: 20.3% (92283 of 452209) allocation can be saved

Comment 1

[André Bargull [:anba]]

Attachment: bug1490985.patch

We can return the base string when the string index (saved in |temp2_|) is zero and the string length (saved in |temp1_|) is equal to the length of the base string. And for Latin-1 encoding, we can return a string from 
|js::StaticStrings::unitStaticTable| when the length is exactly one.

Comment 2

[André Bargull [:anba]]

Attachment: crude-regexp-match-counter-not-for-checkin.patch

Patch used to gather the results in comment #0.

Comment 3

[Jan de Mooij [:jandem]]

Comment on attachment 9008717 [details] [diff] [review]
bug1490985.patch

Review of attachment 9008717 [details] [diff] [review]:
-----------------------------------------------------------------

Awesome results. Thanks!

::: js/src/jit/CodeGenerator.cpp
@@ +1862,5 @@
>                                        ? JSThinInlineString::MAX_LENGTH_LATIN1
>                                        : JSThinInlineString::MAX_LENGTH_TWO_BYTE;
>          masm.branch32(Assembler::Above, temp1_, Imm32(maxThinInlineLength), &fatInline);
> +        if (encoding_ == CharEncoding::Latin1) {
> +            Label thinInline;

Maybe add a brief comment above this, something like your comment 1: "For Latin-1 encoding, we can return a string from |js::StaticStrings::unitStaticTable| when the length is exactly one."

Comment 4

[André Bargull [:anba]]

Attachment: bug1490985.patch

Applied review comments, carrying r+.

Comment 5

[André Bargull [:anba]]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9fca76823a41d588c46458fb485c4063cf843d3a

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/501a1147cb5f
Return input string or static strings in CreateDependentString when possible. r=jandem

Comment 7

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/501a1147cb5f