Use simple loop tracking instead of size to determine if

Use simple loop tracking instead of size to determine if
speculative type assumptions should be used.

This seems like a better indication of hotness than the previous size + use count estimate.

R=ngeoffray@google.com
BUG=
TEST=

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

[ngeoffray, 2012-06-12 09:17:40]

Indeed, looks like a much better heuristic. There are a few things to change in
the code though.

https://chromiumcodereview.appspot.com/10534109/diff/2001/lib/compiler/implem...
File lib/compiler/implementation/elements/elements.dart (right):

https://chromiumcodereview.appspot.com/10534109/diff/2001/lib/compiler/implem...
lib/compiler/implementation/elements/elements.dart:544: bool calledInLoop =
false;
This doesn't belong here. An element is something where most things can be final
(Node and Type are not final because they are computed lazily).

You can save such information in the tasks themselves (eg the optimizer task),
with a set of elements or a set of selectors.

https://chromiumcodereview.appspot.com/10534109/diff/2001/lib/compiler/implem...
File lib/compiler/implementation/enqueue.dart (right):

https://chromiumcodereview.appspot.com/10534109/diff/2001/lib/compiler/implem...
lib/compiler/implementation/enqueue.dart:152: [bool inLoop]) {
I wouldn't let the enqueuer be aware of such considerations. It's really
something only the backend needs to care.

[Mads Ager (google), 2012-06-12 11:01:56]

Thanks for the comments Nicolas! Please take another look.

https://chromiumcodereview.appspot.com/10534109/diff/2001/lib/compiler/implem...
File lib/compiler/implementation/elements/elements.dart (right):

https://chromiumcodereview.appspot.com/10534109/diff/2001/lib/compiler/implem...
lib/compiler/implementation/elements/elements.dart:544: bool calledInLoop =
false;
On 2012/06/12 09:17:40, ngeoffray wrote:
> This doesn't belong here. An element is something where most things can be
final
> (Node and Type are not final because they are computed lazily).
> 
> You can save such information in the tasks themselves (eg the optimizer task),
> with a set of elements or a set of selectors.

Thanks Nicolas! That makes a lot of sense. I'll collect this information in the
builder instead and use it there. That will actually allow it to hit in
situation where it cannot at this point.

https://chromiumcodereview.appspot.com/10534109/diff/2001/lib/compiler/implem...
File lib/compiler/implementation/enqueue.dart (right):

https://chromiumcodereview.appspot.com/10534109/diff/2001/lib/compiler/implem...
lib/compiler/implementation/enqueue.dart:152: [bool inLoop]) {
On 2012/06/12 09:17:40, ngeoffray wrote:
> I wouldn't let the enqueuer be aware of such considerations. It's really
> something only the backend needs to care.

Good point. Removed.

[ngeoffray, 2012-06-12 11:11:09]

LGTM, but I think you should take the whole selector (the Selector class),
instead of just using the name.

https://chromiumcodereview.appspot.com/10534109/diff/8/lib/compiler/implement...
File lib/compiler/implementation/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/10534109/diff/8/lib/compiler/implement...
lib/compiler/implementation/ssa/builder.dart:144: final Set<SourceString>
selectorsCalledInLoop;
I think this should be a Map<SourceString, Selector>, so that you only optimize
a method that can actually be hit by the call in a loop (eg matches the number
of arugments, or is of the right type, or ...).

https://chromiumcodereview.appspot.com/10534109/diff/8/lib/compiler/implement...
File lib/compiler/implementation/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/10534109/diff/8/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:1610: var inLoop =
node.block.enclosingLoopHeader !== null;
var -> bool

https://chromiumcodereview.appspot.com/10534109/diff/8/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:1615:
backend.builder.functionsCalledInLoop.add(node.element);
You should store the optimized selector (line 1624) instead. See my comment in
builder.dart.

[Mads Ager (google), 2012-06-12 11:24:21]

Thanks!

https://chromiumcodereview.appspot.com/10534109/diff/8/lib/compiler/implement...
File lib/compiler/implementation/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/10534109/diff/8/lib/compiler/implement...
lib/compiler/implementation/ssa/builder.dart:144: final Set<SourceString>
selectorsCalledInLoop;
On 2012/06/12 11:11:09, ngeoffray wrote:
> I think this should be a Map<SourceString, Selector>, so that you only
optimize
> a method that can actually be hit by the call in a loop (eg matches the number
> of arugments, or is of the right type, or ...).

Another good point. Done!

https://chromiumcodereview.appspot.com/10534109/diff/8/lib/compiler/implement...
File lib/compiler/implementation/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/10534109/diff/8/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:1610: var inLoop =
node.block.enclosingLoopHeader !== null;
On 2012/06/12 11:11:09, ngeoffray wrote:
> var -> bool

Done.

https://chromiumcodereview.appspot.com/10534109/diff/8/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:1615:
backend.builder.functionsCalledInLoop.add(node.element);
On 2012/06/12 11:11:09, ngeoffray wrote:
> You should store the optimized selector (line 1624) instead. See my comment in
> builder.dart.

Done.