dart2js: add function coverage tracking in dart2js output, dumpinfo, and

dart2js: add function coverage tracking in dart2js output, dumpinfo, and
coverage recording server

R=sra@google.com

Committed: https://github.com/dart-lang/sdk/commit/e3fbf4464383ba55bdaf0619b9bb415e4f1ad8db

[Siggi Cherem (dart-lang), 2015-08-12 01:19:46]

sigmund@google.com changed reviewers:
+ het@google.com, johnniwinther@google.com, sra@google.com

[Siggi Cherem (dart-lang), 2015-08-12 01:19:47]

I'll send separately a tool that can process the data produced by the coverage
log server.

https://codereview.chromium.org/1288593002/diff/1/pkg/compiler/tool/coverage_...
File pkg/compiler/tool/coverage_log_server.dart (right):

https://codereview.chromium.org/1288593002/diff/1/pkg/compiler/tool/coverage_...
pkg/compiler/tool/coverage_log_server.dart:52: htmlPath = args.rest[1];
Harry - I run into some other bugs in the serving and redirecting below, as part
of the process I ended up adding support for providing the html file like you
needed for angular's hello world.

[Siggi Cherem (dart-lang), 2015-08-13 01:38:56]

I just updated this to properly add the coverage tracing call. Stephen found the
location where this has been done before, so I did a slight adaptation to
include then new kind of tracing.

https://codereview.chromium.org/1288593002/diff/20001/sdk/lib/_internal/js_ru...
File sdk/lib/_internal/js_runtime/lib/js_helper.dart (right):

https://codereview.chromium.org/1288593002/diff/20001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/js_helper.dart:278: JS('', '# = []',
JS_EMBEDDED_GLOBAL('JSArray', TRACE_BUFFER));
is there a better way?

I tried defining here a top-level, but because we inject these calls very early
on (even for internal dart2js functions), we hit the issue that the lazy
top-level is not defined yet.

[Siggi Cherem (dart-lang), 2015-08-13 20:52:29]

https://codereview.chromium.org/1288593002/diff/20001/sdk/lib/_internal/js_ru...
File sdk/lib/_internal/js_runtime/lib/js_helper.dart (right):

https://codereview.chromium.org/1288593002/diff/20001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/js_helper.dart:278: JS('', '# = []',
JS_EMBEDDED_GLOBAL('JSArray', TRACE_BUFFER));
On 2015/08/13 01:38:56, Siggi Cherem (dart-lang) wrote:
> is there a better way?
> 
> I tried defining here a top-level, but because we inject these calls very
early
> on (even for internal dart2js functions), we hit the issue that the lazy
> top-level is not defined yet.

As discussed in person, I switched to use an uninitialized top-level. I added a
comment explaining the issue I was hitting before.

[sra1, 2015-08-13 22:27:26]

lgtm

https://codereview.chromium.org/1288593002/diff/40001/pkg/compiler/lib/src/du...
File pkg/compiler/lib/src/dump_info.dart (right):

https://codereview.chromium.org/1288593002/diff/40001/pkg/compiler/lib/src/du...
pkg/compiler/lib/src/dump_info.dart:131: coverageId: '${element.hashCode}',
Add comment that this is unique.

https://codereview.chromium.org/1288593002/diff/40001/pkg/compiler/lib/src/in...
File pkg/compiler/lib/src/info/info.dart (right):

https://codereview.chromium.org/1288593002/diff/40001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/info/info.dart:30: // TODO(sigmund): unify all ids.
What do you mean by unify?

https://codereview.chromium.org/1288593002/diff/40001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/info/info.dart:79: // TODO(sigmund): omit this also when
outputUnit.id == 0
nit: TODOs should have sentences. Begin with 'O', end with a period.

https://codereview.chromium.org/1288593002/diff/40001/sdk/lib/_internal/js_ru...
File sdk/lib/_internal/js_runtime/lib/js_helper.dart (right):

https://codereview.chromium.org/1288593002/diff/40001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/js_helper.dart:292: }, 1000)''', _traceBuffer,
_traceBuffer);
Don't use # inside a JS function.
Use the a surrounding function to bind the Dart expressions to JS locals:

   JS('', r'''
      (function(traceBuffer) {
        window.setTimeout(
          function () {
            var xhr = new XMLHttpRequest();
            xhr.open("POST", "/coverage_uri_to_amend_by_server");
            xhr.send(JSON.stringify(traceBuffer));
            traceBuffer.length = 0;
          },
          1000);
       })(#)''', _traceBuffer);

[Siggi Cherem (dart-lang), 2015-08-13 23:40:21]

thanks Stephen!

https://codereview.chromium.org/1288593002/diff/40001/pkg/compiler/lib/src/du...
File pkg/compiler/lib/src/dump_info.dart (right):

https://codereview.chromium.org/1288593002/diff/40001/pkg/compiler/lib/src/du...
pkg/compiler/lib/src/dump_info.dart:131: coverageId: '${element.hashCode}',
On 2015/08/13 22:27:26, sra1 wrote:
> Add comment that this is unique.

Done.

https://codereview.chromium.org/1288593002/diff/40001/pkg/compiler/lib/src/in...
File pkg/compiler/lib/src/info/info.dart (right):

https://codereview.chromium.org/1288593002/diff/40001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/info/info.dart:30: // TODO(sigmund): unify all ids.
On 2015/08/13 22:27:26, sra1 wrote:
> What do you mean by unify?

since we also have id and serializedId, it would be nice to have a single id for
all of them. I clarified the TODO

https://codereview.chromium.org/1288593002/diff/40001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/info/info.dart:79: // TODO(sigmund): omit this also when
outputUnit.id == 0
On 2015/08/13 22:27:26, sra1 wrote:
> nit: TODOs should have sentences. Begin with 'O', end with a period.

Done.

https://codereview.chromium.org/1288593002/diff/40001/sdk/lib/_internal/js_ru...
File sdk/lib/_internal/js_runtime/lib/js_helper.dart (right):

https://codereview.chromium.org/1288593002/diff/40001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/js_helper.dart:292: }, 1000)''', _traceBuffer,
_traceBuffer);
On 2015/08/13 22:27:26, sra1 wrote:
> Don't use # inside a JS function.
> Use the a surrounding function to bind the Dart expressions to JS locals:
> 
>    JS('', r'''
>       (function(traceBuffer) {
>         window.setTimeout(
>           function () {
>             var xhr = new XMLHttpRequest();
>             xhr.open("POST", "/coverage_uri_to_amend_by_server");
>             xhr.send(JSON.stringify(traceBuffer));
>             traceBuffer.length = 0;
>           },
>           1000);
>        })(#)''', _traceBuffer);

Done.

[Siggi Cherem (dart-lang), 2015-08-13 23:48:03]

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

[Johnni Winther, 2015-08-19 10:38:22]

lgtm

https://codereview.chromium.org/1288593002/diff/60001/pkg/compiler/tool/cover...
File pkg/compiler/tool/coverage_log_server.dart (right):

https://codereview.chromium.org/1288593002/diff/60001/pkg/compiler/tool/cover...
pkg/compiler/tool/coverage_log_server.dart:123: ? '<html><script
src="$baseJsName"></script>'
Is the missing </html> deliberate?

[Siggi Cherem (dart-lang), 2015-08-19 15:45:20]

https://codereview.chromium.org/1288593002/diff/60001/pkg/compiler/tool/cover...
File pkg/compiler/tool/coverage_log_server.dart (right):

https://codereview.chromium.org/1288593002/diff/60001/pkg/compiler/tool/cover...
pkg/compiler/tool/coverage_log_server.dart:123: ? '<html><script
src="$baseJsName"></script>'
On 2015/08/19 10:38:22, Johnni Winther wrote:
> Is the missing </html> deliberate?

yeah, but it is equivalent (HTML parsing spec says it's optional).