Closed
Bug 1119081
Opened 10 years ago
Closed 10 years ago
Use JSInlineString where possible when concatenating strings in JITted code
Categories
(Core :: JavaScript Engine: JIT, defect)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla37
People
(Reporter: n.nethercote, Assigned: jandem)
Details
Attachments
(1 file)
(deleted),
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
Bug 1105895 changed ConcatString() to use a non-fat inline string where possible. We should do the same for JITted concatenation code. Jan said:
> Also, the JIT inlines this code in JitCompartment::generateStringConcatStub, we should
> probably do the same thing there to be consistent. It seems like you can do this in
> ConcatFatInlineString, the concatenated length is in temp2 at the start of that
> function, so we should be able to branch on that.
Reporter | ||
Updated•10 years ago
|
Summary: Use JSInlineString where possible in ConcatStrings() → Use JSInlineString where possible when concatenating strings in JITted code
Assignee | ||
Comment 1•10 years ago
|
||
This matches what njn is doing for the ConcatStrings VM function in bug 1105895 and JIT code should behave the same to avoid benchmark regressions (see bug 1105895 comment 15 for the details).
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Attachment #8546676 -
Flags: review?(bhackett1024)
Updated•10 years ago
|
Attachment #8546676 -
Flags: review?(bhackett1024) → review+
Assignee | ||
Comment 2•10 years ago
|
||
Comment 3•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Comment 4•10 years ago
|
||
@Jan: I didn't look into what changes are happening, but do we need a similar change for "visitSubstr"? Since we create strings in assembly in LSubstr too? I think not, but just asking to be sure we don't miss that.
Updated•10 years ago
|
Flags: needinfo?(jdemooij)
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to Hannes Verschore [:h4writer] from comment #4)
> @Jan: I didn't look into what changes are happening, but do we need a
> similar change for "visitSubstr"? Since we create strings in assembly in
> LSubstr too? I think not, but just asking to be sure we don't miss that.
Good point. The code we emit in visitSubstr always creates a FatInlineString in the inline case. JSDependentString::new_ creates a fat inline or non-fat inline string. So it seems the JIT code there needs the same fix.
If we have LSubstr (with a very short result string) in a loop, the current code should be measurably slower. Do you want to file/fix?
Btw, the code in CreateDependentString does handle this, could we use that somehow?
Flags: needinfo?(jdemooij)
You need to log in
before you can comment on or make changes to this bug.
Description
•