Sort class IDs before training AppJIT snapshots

Sort class IDs before training AppJIT snapshots

R=fschneider@google.com, rmacnak@google.com
BUG=

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

[Florian Schneider, 2017-02-16 09:08:34]

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

[Florian Schneider, 2017-02-16 09:08:34]

+Ryan

[Florian Schneider, 2017-02-16 10:05:07]

Lgtm.

Please run tests with -capp_jit -mdebug,release,product

https://codereview.chromium.org/2699853002/diff/1/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

https://codereview.chromium.org/2699853002/diff/1/runtime/include/dart_api.h#...
runtime/include/dart_api.h:3286: DART_EXPORT void
Formatting: One line?

https://codereview.chromium.org/2699853002/diff/1/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/2699853002/diff/1/runtime/vm/dart_api_impl.cc...
runtime/vm/dart_api_impl.cc:6609: DART_EXPORT void
Formatting: One line?

[rmacnak, 2017-02-16 17:19:20]

lgtm w/c

Commit message: when writing snapshots -> before training snapshots

https://codereview.chromium.org/2699853002/diff/1/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/2699853002/diff/1/runtime/vm/dart_api_impl.cc...
runtime/vm/dart_api_impl.cc:6613: }
ClearAllCode

[erikcorry, 2017-02-20 09:37:26]

Description was changed from

==========
Sort class IDs when writing AppJIT snapshots

R=fschneider@google.com
BUG=
==========

to

==========
Sort class IDs before training AppJIT snapshots

R=fschneider@google.com
BUG=
==========

[erikcorry, 2017-02-20 11:14:25]

Description was changed from

==========
Sort class IDs before training AppJIT snapshots

R=fschneider@google.com
BUG=
==========

to

==========
Sort class IDs before training AppJIT snapshots

R=fschneider@google.com, rmacnak@google.com
BUG=

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

[erikcorry, 2017-02-20 11:14:27]

Committed patchset #2 (id:20001) manually as
c6a199e40b0fa67eef293bdef52e897264e0ef7f (presubmit successful).

[erikcorry, 2017-02-20 12:35:19]

https://codereview.chromium.org/2699853002/diff/1/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

https://codereview.chromium.org/2699853002/diff/1/runtime/include/dart_api.h#...
runtime/include/dart_api.h:3286: DART_EXPORT void
On 2017/02/16 10:05:06, Florian Schneider wrote:
> Formatting: One line?

Done.

https://codereview.chromium.org/2699853002/diff/1/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/2699853002/diff/1/runtime/vm/dart_api_impl.cc...
runtime/vm/dart_api_impl.cc:6609: DART_EXPORT void
On 2017/02/16 10:05:06, Florian Schneider wrote:
> Formatting: One line?

Done.

https://codereview.chromium.org/2699853002/diff/1/runtime/vm/dart_api_impl.cc...
runtime/vm/dart_api_impl.cc:6613: }
On 2017/02/16 17:19:20, rmacnak wrote:
> ClearAllCode

Done.

[zra, 2017-02-20 16:59:17]

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

[zra, 2017-02-20 16:59:17]

DBC

Should Dart_SortClasses *really* be part of the public API?

Couldn't it at least just be an optional part of some other call?

[zra, 2017-02-21 17:39:05]

On 2017/02/20 16:59:17, zra wrote:
> DBC
> 
> Should Dart_SortClasses *really* be part of the public API?
> 
> Couldn't it at least just be an optional part of some other call?

I think this should try to piggy-back on Dart_FinalizeLoading() defined here:

https://github.com/dart-lang/sdk/blob/master/runtime/vm/dart_api_impl.cc#L5850

instead of adding a new call.

Would it also be possible to add a test that checks that the classes are
actually sorted before the first compile happens?