Open Bug 856178 Opened 11 years ago Updated 2 years ago

IonMonkey: Optimize append operations on strings.


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





(Reporter: nbp, Unassigned)


(Blocks 1 open bug)



(2 files)

In PdfJS octane benchmark we can notice the following loop:

    var eexecStr = '';
    for (var i = 0, ii = eexec.length; i < ii; i++)
      eexecStr += String.fromCharCode(eexec[i]);

which also exists in different variants where the right hand side is likely a one-character string.  Such string is then either constantly read with eexecStr[i] or compared against constants.  Which means that we are currently creating a linked list of characters (ropes), and then we linearized everything for reading from the string.

At the moment, IonMonkey does not inline the string concat operation.  If we can ensure that nothing will hold the string in one state, we can use an extensible string and inline the concatenation operations with an amortized log case for the reallocation of the string buffer.

Note: Also, this kind of operations is likely to become more common at the interface boundaries of asm.js functions.
Attached file Micro benchmark
Attachment #731713 - Attachment is patch: false
This patch check that a string buffer is not escaped and optimize loops of append by allocating an extensible array and replace concatenations by appends.

This optimization is made to be equivalent to the result of a rope creation which is then flattened.  The only trick is that we should avoid other flatten to prevent our extensible buffer to be converted to a dependent string.

@luke: Can you check if the string logic is correct there or if I missed some corner cases?
Attachment #731715 - Flags: feedback?(luke)
Depends on: 856533
Comment on attachment 731715 [details] [diff] [review]
Append characters to extensible strings in loops.

The concept makes sense.  The only worrying thing is the size of the patch, so it'd be good to get a confirmation of how much this helps Octane or how often it matches real code (or Emscripten glue code, per comment 0).
Attachment #731715 - Flags: feedback?(luke) → feedback+
Blocks: 885526
Assignee: nicolas.b.pierron → nobody
Component: JavaScript Engine → JavaScript Engine: JIT
Priority: -- → P5
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.