Use growable lists.

Use growable lists.

BUG=http://dartbug.com/15599
R=johnniwinther@google.com

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

[karlklose, 2013-12-12 07:57:17]

DBC:

https://codereview.chromium.org/113283008/diff/1/dart/sdk/lib/_internal/compi...
File dart/sdk/lib/_internal/compiler/implementation/mirrors_used.dart (right):

https://codereview.chromium.org/113283008/diff/1/dart/sdk/lib/_internal/compi...
dart/sdk/lib/_internal/compiler/implementation/mirrors_used.dart:458:
List<String> result = new List<String>(list.length);
Why not initialize result with '<String>[]' and then add the elements?

https://codereview.chromium.org/113283008/diff/1/dart/sdk/lib/_internal/compi...
dart/sdk/lib/_internal/compiler/implementation/mirrors_used.dart:461:
assert(invariant(spannable, entry is String));
Why this invariant? Checked mode will already check the added element's type.

[ahe, 2013-12-12 09:58:07]

https://codereview.chromium.org/113283008/diff/1/dart/sdk/lib/_internal/compi...
File dart/sdk/lib/_internal/compiler/implementation/mirrors_used.dart (right):

https://codereview.chromium.org/113283008/diff/1/dart/sdk/lib/_internal/compi...
dart/sdk/lib/_internal/compiler/implementation/mirrors_used.dart:458:
List<String> result = new List<String>(list.length);
On 2013/12/12 07:57:18, karlklose wrote:
> Why not initialize result with '<String>[]' and then add the elements?

I know the size I need. I couldn't find any way to create a growable array with
a pre-allocated size.

https://codereview.chromium.org/113283008/diff/1/dart/sdk/lib/_internal/compi...
dart/sdk/lib/_internal/compiler/implementation/mirrors_used.dart:461:
assert(invariant(spannable, entry is String));
On 2013/12/12 07:57:18, karlklose wrote:
> Why this invariant? Checked mode will already check the added element's type.

Checked mode doesn't know which AST node (or element) is causing the crash.
Also, checked mode allows null.

I think I can change this method to:

if (list == null) return null;
assert(invariant(spannable, list is List<String> && list is! List<int>));
assert(invariant(spannable, !list.contains(null));
return list;

Provided I also change convertConstantToUsageList to always return a
List<String>.

[karlklose, 2013-12-12 10:27:49]

https://codereview.chromium.org/113283008/diff/1/dart/sdk/lib/_internal/compi...
File dart/sdk/lib/_internal/compiler/implementation/mirrors_used.dart (right):

https://codereview.chromium.org/113283008/diff/1/dart/sdk/lib/_internal/compi...
dart/sdk/lib/_internal/compiler/implementation/mirrors_used.dart:458:
List<String> result = new List<String>(list.length);
On 2013/12/12 09:58:07, ahe wrote:
> On 2013/12/12 07:57:18, karlklose wrote:
> > Why not initialize result with '<String>[]' and then add the elements?
> 
> I know the size I need. I couldn't find any way to create a growable array
with
> a pre-allocated size.

toList will create a new List and add the elements anyway, so this is not an
optimization.  I think the code reads better (and is faster), if you used the
literal and add.

https://codereview.chromium.org/113283008/diff/1/dart/sdk/lib/_internal/compi...
dart/sdk/lib/_internal/compiler/implementation/mirrors_used.dart:464: return
result.toList(growable: true);
'true' is the default.

[ahe, 2013-12-12 11:04:58]

I have removed convertToListOfStrings, PTAL.

https://codereview.chromium.org/113283008/diff/1/dart/sdk/lib/_internal/compi...
File dart/sdk/lib/_internal/compiler/implementation/mirrors_used.dart (right):

https://codereview.chromium.org/113283008/diff/1/dart/sdk/lib/_internal/compi...
dart/sdk/lib/_internal/compiler/implementation/mirrors_used.dart:458:
List<String> result = new List<String>(list.length);
On 2013/12/12 10:27:49, karlklose wrote:
> On 2013/12/12 09:58:07, ahe wrote:
> > On 2013/12/12 07:57:18, karlklose wrote:
> > > Why not initialize result with '<String>[]' and then add the elements?
> > 
> > I know the size I need. I couldn't find any way to create a growable array
> with
> > a pre-allocated size.
> 
> toList will create a new List and add the elements anyway, so this is not an
> optimization.  I think the code reads better (and is faster), if you used the
> literal and add.

That sounds like a performance bug in toList.

https://codereview.chromium.org/113283008/diff/1/dart/sdk/lib/_internal/compi...
dart/sdk/lib/_internal/compiler/implementation/mirrors_used.dart:464: return
result.toList(growable: true);
On 2013/12/12 10:27:49, karlklose wrote:
> 'true' is the default.

Yes. But I'm passing the value explicitly to document that I want a growable
list.

[ahe, 2014-01-07 11:14:49]

ping

[Johnni Winther, 2014-01-07 11:35:03]

lgtm

[ahe, 2014-01-09 14:30:07]

Committed patchset #4 manually as r31659 (presubmit successful).