Better dependency tracking

This CL lands better dependency tracking by storing the selectors generated by function elements and re-applying them to the global list of functions at the dump-info stage.

In a future CL, dump-info will analyze the selectors and create human-readable "reasons" that the selector was made.

BUG=
R=sra@google.com

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

[Ty Overby (Google), 2014-07-30 19:56:08]

https://codereview.chromium.org/430913002/diff/20001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/430913002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/compiler.dart:91: 
Remove blank line.

https://codereview.chromium.org/430913002/diff/20001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/dependency_info.dart (right):

https://codereview.chromium.org/430913002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/dependency_info.dart:26: return
<Element>[];
Make const.

https://codereview.chromium.org/430913002/diff/20001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/enqueue.dart (right):

https://codereview.chromium.org/430913002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/enqueue.dart:75: //
dependencyInfo.addsToWorkList(compiler.currentElement, element);
Remove.

https://codereview.chromium.org/430913002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/enqueue.dart:192:
//dependencyInfo.enqueues(getContext(), member);
Remove

https://codereview.chromium.org/430913002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/enqueue.dart:482:
//dependencyInfo.enqueues(context, member);
Remove

[Ty Overby (Google), 2014-07-30 19:59:54]

https://codereview.chromium.org/430913002/diff/20001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/430913002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/compiler.dart:91: 
On 2014/07/30 19:56:08, Ty Overby (Google) wrote:
> Remove blank line.

Done.

https://codereview.chromium.org/430913002/diff/20001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/dependency_info.dart (right):

https://codereview.chromium.org/430913002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/dependency_info.dart:26: return
<Element>[];
On 2014/07/30 19:56:08, Ty Overby (Google) wrote:
> Make const.

Done.

https://codereview.chromium.org/430913002/diff/20001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/enqueue.dart (right):

https://codereview.chromium.org/430913002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/enqueue.dart:75: //
dependencyInfo.addsToWorkList(compiler.currentElement, element);
On 2014/07/30 19:56:08, Ty Overby (Google) wrote:
> Remove.

Done.

https://codereview.chromium.org/430913002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/enqueue.dart:192:
//dependencyInfo.enqueues(getContext(), member);
On 2014/07/30 19:56:08, Ty Overby (Google) wrote:
> Remove

Done.

https://codereview.chromium.org/430913002/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/enqueue.dart:482:
//dependencyInfo.enqueues(context, member);
On 2014/07/30 19:56:08, Ty Overby (Google) wrote:
> Remove

Done.

[sra1, 2014-07-30 21:43:48]

Let's see if we can make the coupling between the DumpInfoTask and the rest of
the compiler happen in CodegenRegistry rather than in Enqueuer.

https://chromiumcodereview.appspot.com/430913002/diff/40001/sdk/lib/_internal...
File sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://chromiumcodereview.appspot.com/430913002/diff/40001/sdk/lib/_internal...
sdk/lib/_internal/compiler/implementation/compiler.dart:56:
DependencyInformation dependencyInfo;
Allocate it (the only DependencyInformation) here, or have the DumpInfoTask own
it.

https://chromiumcodereview.appspot.com/430913002/diff/40001/sdk/lib/_internal...
sdk/lib/_internal/compiler/implementation/compiler.dart:90:
world.registerDynamicInvocation(currentElement, selector);
dependencyInfo.elementUsesSelector(currentElement, selector);

https://chromiumcodereview.appspot.com/430913002/diff/40001/sdk/lib/_internal...
sdk/lib/_internal/compiler/implementation/compiler.dart:94:
world.registerDynamicSetter(currentElement, selector);
dependencyInfo.elementUsesSelector(currentElement, selector);

https://chromiumcodereview.appspot.com/430913002/diff/40001/sdk/lib/_internal...
sdk/lib/_internal/compiler/implementation/compiler.dart:98:
world.registerDynamicGetter(currentElement, selector);
dependencyInfo.elementUsesSelector(currentElement, selector);

https://chromiumcodereview.appspot.com/430913002/diff/40001/sdk/lib/_internal...
File sdk/lib/_internal/compiler/implementation/dependency_info.dart (right):

https://chromiumcodereview.appspot.com/430913002/diff/40001/sdk/lib/_internal...
sdk/lib/_internal/compiler/implementation/dependency_info.dart:17: void
elementUsesSelector(Element element, Selector selector) {}
abstract methods are declared without a body:

void elementUsesSelector(Element element, Selector selector);

https://chromiumcodereview.appspot.com/430913002/diff/40001/sdk/lib/_internal...
File sdk/lib/_internal/compiler/implementation/enqueue.dart (right):

https://chromiumcodereview.appspot.com/430913002/diff/40001/sdk/lib/_internal...
sdk/lib/_internal/compiler/implementation/enqueue.dart:52: DependencyInformation
get dependencyInfo;
I would remove this from the enqueuers and do it all in CodegenRegistry.

https://chromiumcodereview.appspot.com/430913002/diff/40001/sdk/lib/_internal...
sdk/lib/_internal/compiler/implementation/enqueue.dart:260:
dependencyInfo.elementUsesSelector(context, selector);
If we move this to CodegenRegistry, we can get rid of the [context] parameters
in these functions.

[Ty Overby (Google), 2014-08-01 16:39:38]

https://codereview.chromium.org/430913002/diff/40001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/430913002/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/compiler.dart:56:
DependencyInformation dependencyInfo;
On 2014/07/30 21:43:48, sra1 wrote:
> Allocate it (the only DependencyInformation) here, or have the DumpInfoTask
own
> it.
Done.
I got rid of DependencyInformation and use DumpInfoTask.

https://codereview.chromium.org/430913002/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/compiler.dart:90:
world.registerDynamicInvocation(currentElement, selector);
On 2014/07/30 21:43:48, sra1 wrote:
> dependencyInfo.elementUsesSelector(currentElement, selector);

Done.

https://codereview.chromium.org/430913002/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/compiler.dart:94:
world.registerDynamicSetter(currentElement, selector);
On 2014/07/30 21:43:48, sra1 wrote:
> dependencyInfo.elementUsesSelector(currentElement, selector);

Done.

https://codereview.chromium.org/430913002/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/compiler.dart:98:
world.registerDynamicGetter(currentElement, selector);
On 2014/07/30 21:43:48, sra1 wrote:
> dependencyInfo.elementUsesSelector(currentElement, selector);

Done.

https://codereview.chromium.org/430913002/diff/40001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/dependency_info.dart (right):

https://codereview.chromium.org/430913002/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/dependency_info.dart:17: void
elementUsesSelector(Element element, Selector selector) {}
On 2014/07/30 21:43:48, sra1 wrote:
> abstract methods are declared without a body:
> 
> void elementUsesSelector(Element element, Selector selector);
This class is gone.  
Done.

https://codereview.chromium.org/430913002/diff/40001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/enqueue.dart (right):

https://codereview.chromium.org/430913002/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/enqueue.dart:52: DependencyInformation
get dependencyInfo;
On 2014/07/30 21:43:48, sra1 wrote:
> I would remove this from the enqueuers and do it all in CodegenRegistry.

Done.

https://codereview.chromium.org/430913002/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/enqueue.dart:260:
dependencyInfo.elementUsesSelector(context, selector);
On 2014/07/30 21:43:48, sra1 wrote:
> If we move this to CodegenRegistry, we can get rid of the [context] parameters
> in these functions.

Done.

[Ty Overby (Google), 2014-08-01 16:54:51]

https://codereview.chromium.org/430913002/diff/60001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/dump_info.dart (right):

https://codereview.chromium.org/430913002/diff/60001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/dump_info.dart:124: String
idOf(Element element) {
Document this function.

https://codereview.chromium.org/430913002/diff/60001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/dump_info.dart:372: * Ret
Finish documenting this function you fool.

https://codereview.chromium.org/430913002/diff/60001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/dump_info.dart:502: for (Element fn in
infoCollector.mapper.functions) {
Comment this.

https://codereview.chromium.org/430913002/diff/60001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/enqueue.dart (right):

https://codereview.chromium.org/430913002/diff/60001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/enqueue.dart:250: void
registerNewSelector(Selector selector, Map<String, Set<Selector>> selectorsMap)
{
Line length.

[Ty Overby (Google), 2014-08-01 17:23:18]

https://codereview.chromium.org/430913002/diff/60001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/dump_info.dart (right):

https://codereview.chromium.org/430913002/diff/60001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/dump_info.dart:124: String
idOf(Element element) {
On 2014/08/01 16:54:50, Ty Overby (Google) wrote:
> Document this function.

Done.

https://codereview.chromium.org/430913002/diff/60001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/dump_info.dart:366:
selectorsFromElement
Only perform this when --dump-info is provided.

Done.

https://codereview.chromium.org/430913002/diff/60001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/dump_info.dart:372: * Ret
On 2014/08/01 16:54:50, Ty Overby (Google) wrote:
> Finish documenting this function you fool.

Done.

https://codereview.chromium.org/430913002/diff/60001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/dump_info.dart:502: for (Element fn in
infoCollector.mapper.functions) {
On 2014/08/01 16:54:50, Ty Overby (Google) wrote:
> Comment this.

Done.

https://codereview.chromium.org/430913002/diff/60001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/enqueue.dart (right):

https://codereview.chromium.org/430913002/diff/60001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/enqueue.dart:250: void
registerNewSelector(Selector selector, Map<String, Set<Selector>> selectorsMap)
{
On 2014/08/01 16:54:50, Ty Overby (Google) wrote:
> Line length.

Done.

[sra1, 2014-08-01 18:29:32]

lgtm

LGTM.
The bigger comments are for the future.

https://codereview.chromium.org/430913002/diff/100001/sdk/lib/_internal/compi...
File sdk/lib/_internal/compiler/implementation/dump_info.dart (right):

https://codereview.chromium.org/430913002/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/dump_info.dart:381: if
(!selectorsFromElement.containsKey(element)) {
Usually .containsKey and operator[] take about the same time, so if you know
that the map never has `null` values, this is a slightly more efficient pattern:

Set<Selector> selectors = selectorsFromElement[element];
if (selectors == null) {
  return const ...;
} else {
  return selectors.expand(...)
}

https://codereview.chromium.org/430913002/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/dump_info.dart:385: (s) =>
compiler.world.allFunctions.filter(s));
This will have duplicate entries when multiple selectors match the same
function.  (Selectors Iterable.length and List.length both hit the
JSArray.length implementation).

Is this what we want?

This is OK for now but we probably want the JSON to contain enough information
so we can do this calculation in the viewer.

https://codereview.chromium.org/430913002/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/dump_info.dart:513: // recorded. 
Don't regester these.
s/regester/register/

https://codereview.chromium.org/430913002/diff/100001/sdk/lib/_internal/compi...
File sdk/lib/_internal/compiler/implementation/resolution/registry.dart (right):

https://codereview.chromium.org/430913002/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/resolution/registry.dart:22: Element
get currentElement => mapping.currentElement;
Can this be removed?

[Ty Overby (Google), 2014-08-01 20:03:04]

Committed patchset #7 manually as 38839 (presubmit successful).