1125590 - Fold constant strings in MConcat

Status: NEW

(Core :: JavaScript Engine: JIT, defect, P5)

Description

[andy.zsshen]

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/40.0.2214.91 Safari/537.36

Comment 1

[andy.zsshen]

For GVN optimization, we rely on the "foldsTo" function in each MIR to reduce computation.
Currently, "MConcat::foldsTo" tests whether the "rhs()" or "lhs()" is empty constant and returns the
non constant operand. 
We can further test if both inputs are constants and return "MConstant" with the concatenated 
constant strings.

Comment 2

[Jan de Mooij [:jandem]]

Thanks for the report.

Unfortunately this is complicated, because we can't allocate strings in MConcat::foldsTo (we don't have a JSContext* and we could be running on a background thread). Maybe we could hack around that by allocating the string on the main thread after compilation is done (in CodeGenerator::link), but it's likely not worth the complexity..

Comment 3

[Nicolas B. Pierron [:nbp]]

We had an issue like that before, and instead decided to do it as part of the JavaScript parser.

Comment 4

[Hannes Verschore [:h4writer]]

Sounds like a fun project sometimes in the far future. Feel free to take it. Just putting needinfo to remember it.

Comment 5

[Tom S. (please needinfo tschuster)]

object-literal-ext.es6 in six-speed ends up with two constant strings. In think for that case we could fold in the IonBuilder where we should be able to allocate.

Comment 6

[Nicolas B. Pierron [:nbp]]

I once tried to do this.  The main issue is that we run the compilation on a different thread.  When foldsTo is executed, we cannot safely allocate a JSString, and we do not want to lock the main thread for creating it.

I see 2 options:
 (1) Create mirror structures to store the same information, and when linking / codegen, register this information back to compartment associated with the compilation.
 (2) Create a new zone in which we can allocate new object (template object), strings, and others, in a similar way as we do for Parsing tasks, and merge these zones back if the compilation succeeded.

I think the option (2) is the best, as it open the road for later running IonBuilder off-main thread as well.

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

Clear a needinfo that is pending on an inactive user.

Inactive users most likely will not respond; if the missing information is essential and cannot be collected another way, the bug maybe should be closed as INCOMPLETE.

For more information, please visit BugBot documentation.

Comment 8

[Mayank Bansal]

Still relevant?