dart2js implementation of StringBuffer.writeAll that optimizes better

dart2js implementation of StringBuffer.writeAll that optimizes better.

Sometimes we can avoid the StringBuffer object:

    t1 = new P.StringBuffer(leftDelimiter);
    t1.writeAll$2(parts, ", ");
    t1 = t1._contents += rightDelimiter;
    return t1.charCodeAt(0) == 0 ? t1 : t1;
-->
    t1 = P.StringBuffer__writeAll(leftDelimiter, parts, ", ") + rightDelimiter;
    return t1.charCodeAt(0) == 0 ? t1 : t1;

Also updated tests with missing cases.

R=lrn@google.com

Committed: https://code.google.com/p/dart/source/detail?r=45184
Reverted: https://code.google.com/p/dart/source/detail?r=45186

Committed: https://code.google.com/p/dart/source/detail?r=45188

[sra1, 2015-04-15 03:06:01]

sra@google.com changed reviewers:
+ lrn@google.com

[Lasse Reichstein Nielsen, 2015-04-15 05:33:24]

lgtm

https://chromiumcodereview.appspot.com/1086043002/diff/1/sdk/lib/_internal/co...
File sdk/lib/_internal/compiler/js_lib/core_patch.dart (right):

https://chromiumcodereview.appspot.com/1086043002/diff/1/sdk/lib/_internal/co...
sdk/lib/_internal/compiler/js_lib/core_patch.dart:414: _contents =
_writeAll(_contents, objects, separator);
Have you tried making two _writeAll versions, one with and one without a
separator, and split on which one to call right here?
I think the separator will almost always be a literal, so if you inline, it
should be possible to remove one of the branches at the call site.

[sra1, 2015-04-15 21:45:06]

https://chromiumcodereview.appspot.com/1086043002/diff/1/sdk/lib/_internal/co...
File sdk/lib/_internal/compiler/js_lib/core_patch.dart (right):

https://chromiumcodereview.appspot.com/1086043002/diff/1/sdk/lib/_internal/co...
sdk/lib/_internal/compiler/js_lib/core_patch.dart:414: _contents =
_writeAll(_contents, objects, separator);
On 2015/04/15 05:33:23, Lasse Reichstein Nielsen wrote:
> Have you tried making two _writeAll versions, one with and one without a
> separator, and split on which one to call right here?
> I think the separator will almost always be a literal, so if you inline, it
> should be possible to remove one of the branches at the call site.

This is a good idea but it ruins the magic :-(
It is too big to inline without knowing a priori that the code will shrink.
We don't currently have the ability to predict that at inlining time, so it does
not get inlined.

Another problem with the existing optimizer is the ?: to select the version
leaves a residual if(true) in the SSA CFG which interferes with store-forwarding
and other optimizations.
We can't easily remove the vestigial control flow (or we would have done it).

Worth revisiting when the new IR is in use.

[sra1, 2015-04-15 22:02:15]

Committed patchset #2 (id:20001) manually as 45184 (presubmit successful).

[sra1, 2015-04-15 23:13:40]

Committed patchset #4 (id:60001) manually as 45188 (presubmit successful).