core-jit:

core-jit:
  Adjust compilation trace format to be plain text friendly.
  Avoid writing functions we can't lookup in the trace.
  Fix tear off fuzzy matching.

R=zra@google.com

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

[rmacnak, 2017-06-08 01:03:29]

rmacnak@google.com changed reviewers:
+ zra@google.com

[zra, 2017-06-08 03:10:31]

https://codereview.chromium.org/2924013004/diff/20001/runtime/vm/compilation_...
File runtime/vm/compilation_trace.cc (right):

https://codereview.chromium.org/2924013004/diff/20001/runtime/vm/compilation_...
runtime/vm/compilation_trace.cc:78: char* comma1 = FindCharacter(uri, ',',
limit);
Is memchr() portable?

https://codereview.chromium.org/2924013004/diff/20001/runtime/vm/compilation_...
runtime/vm/compilation_trace.cc:79: if (comma1 == NULL) break;
Curly braces for single-line ifs.

https://codereview.chromium.org/2924013004/diff/20001/runtime/vm/compilation_...
runtime/vm/compilation_trace.cc:123: for (intptr_t i = 0; i <
closure_functions.Length(); i++) {
How is closure_functions modified by CompileFunction()? The way this loop is
written it looks like a lot of compilation is being repeated (the loop always
starts from 0 instead of where you left off the last time). Maybe add a comment
to clarify.

https://codereview.chromium.org/2924013004/diff/20001/runtime/vm/compilation_...
runtime/vm/compilation_trace.cc:139: RawObject*
CompilationTraceLoader::CompileTriple(const char* uri_cstr,
A comment about the algorithm used for the fuzzy matching might be useful here.

[rmacnak, 2017-06-08 22:15:09]

https://codereview.chromium.org/2924013004/diff/20001/runtime/vm/compilation_...
File runtime/vm/compilation_trace.cc (right):

https://codereview.chromium.org/2924013004/diff/20001/runtime/vm/compilation_...
runtime/vm/compilation_trace.cc:78: char* comma1 = FindCharacter(uri, ',',
limit);
On 2017/06/08 03:10:30, zra wrote:
> Is memchr() portable?

I think so, but it uses a length that would need to be updated at each step
instead of a constant limit.

https://codereview.chromium.org/2924013004/diff/20001/runtime/vm/compilation_...
runtime/vm/compilation_trace.cc:79: if (comma1 == NULL) break;
On 2017/06/08 03:10:30, zra wrote:
> Curly braces for single-line ifs.

Done.

https://codereview.chromium.org/2924013004/diff/20001/runtime/vm/compilation_...
runtime/vm/compilation_trace.cc:123: for (intptr_t i = 0; i <
closure_functions.Length(); i++) {
On 2017/06/08 03:10:30, zra wrote:
> How is closure_functions modified by CompileFunction()? The way this loop is
> written it looks like a lot of compilation is being repeated (the loop always
> starts from 0 instead of where you left off the last time). Maybe add a
comment
> to clarify.

CompileFunction may append to this list, since we register closure functions in
the after building its parent's flow graph.
Undid this because we're safe as long as we don't cache the length for the loop;
added comment to this effect.

https://codereview.chromium.org/2924013004/diff/20001/runtime/vm/compilation_...
runtime/vm/compilation_trace.cc:139: RawObject*
CompilationTraceLoader::CompileTriple(const char* uri_cstr,
On 2017/06/08 03:10:30, zra wrote:
> A comment about the algorithm used for the fuzzy matching might be useful
here.

Added

[zra, 2017-06-08 22:49:42]

lgtm

[rmacnak, 2017-06-08 23:04:33]

Description was changed from

==========
core-jit:
  Adjust compilation trace format to be plain text friendly.
  Avoid writing functions we can't lookup in the trace.
  Fix tear off fuzzy matching.
==========

to

==========
core-jit:
  Adjust compilation trace format to be plain text friendly.
  Avoid writing functions we can't lookup in the trace.
  Fix tear off fuzzy matching.

R=zra@google.com

Committed:
https://github.com/dart-lang/sdk/commit/812c3423bcf31a89f9a1223e381ce62bd26d77c7
==========

[rmacnak, 2017-06-08 23:04:35]

Committed patchset #3 (id:40001) manually as
812c3423bcf31a89f9a1223e381ce62bd26d77c7 (presubmit successful).