VM: [Kernel] Split kernel API into 3 steps: ([read binary], parse-binary, bootstrap, load program)

VM: [Kernel] Split kernel API into 3 steps: ([read binary], parse-binary, bootstrap, load program)

This avoids reading and parsing the kernel binary multiple times and avois
having it multiple times in memory (we don't free the 'kernel::Program*' object
ATM).

R=kmillikin@google.com

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

[kustermann, 2016-11-22 13:10:34]

kustermann@google.com changed reviewers:
+ kmillikin@google.com, vegorov@google.com

[kustermann, 2016-11-22 13:11:45]

https://codereview.chromium.org/2525623002/diff/1/runtime/bin/loader.cc
File runtime/bin/loader.cc (left):

https://codereview.chromium.org/2525623002/diff/1/runtime/bin/loader.cc#oldco...
runtime/bin/loader.cc:374: dart_result = Dart_LoadKernel(payload,
payload_length);
We cannot hit this code path anymore AFAIK. It was used at the beginning while
we were using the hybrid mode.

[Kevin Millikin (Google), 2016-11-22 15:51:18]

LGTM, nice cleanup.

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

https://codereview.chromium.org/2525623002/diff/1/runtime/include/dart_api.h#...
runtime/include/dart_api.h:2845: * \param kernel_program The
`dart::kernel::Program` object returned by .
Missing the name of the function that returns the program.

https://codereview.chromium.org/2525623002/diff/1/runtime/include/dart_api.h#...
runtime/include/dart_api.h:2856: * Lonstructs a kernel program form a binary.
Lonstructs == Load + constructs?

https://codereview.chromium.org/2525623002/diff/1/runtime/include/dart_api.h#...
runtime/include/dart_api.h:2863: DART_EXPORT void* Dart_ParseKernelBinary(const
uint8_t* buffer,
I don't think of this as parsing.  Deserializing, but maybe that's too long. 
"ReadKernelBinary"?  "LoadKernelBinary"?

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

https://codereview.chromium.org/2525623002/diff/1/runtime/vm/dart.cc#newcode515
runtime/vm/dart.cc:515: if (kernel_program != NULL) {
Here you can just unconditionally call Object::Init(I, kernel_program).

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

https://codereview.chromium.org/2525623002/diff/1/runtime/vm/dart_api_impl.cc...
runtime/vm/dart_api_impl.cc:5387: // free it because (similar to the token
stream) it will be used to run our
"used to repeatedly run the `kernel::FlowGraphBuilder`."

[kustermann, 2016-11-23 08:26:21]

Description was changed from

==========
VM: [Kernel] Split kernel API into 3 steps: ([read binary], parse-binary,
bootstrap, load program)

This avoids reading and parsing the kernel binary multiple times and avois
having it multiple times in memory (we don't free the 'kernel::Program*' object
ATM).
==========

to

==========
VM: [Kernel] Split kernel API into 3 steps: ([read binary], parse-binary,
bootstrap, load program)

This avoids reading and parsing the kernel binary multiple times and avois
having it multiple times in memory (we don't free the 'kernel::Program*' object
ATM).

R=kmillikin@google.com

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

[kustermann, 2016-11-23 08:26:23]

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

[kustermann, 2016-11-23 08:31:46]

Thanks.

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

https://codereview.chromium.org/2525623002/diff/1/runtime/include/dart_api.h#...
runtime/include/dart_api.h:2845: * \param kernel_program The
`dart::kernel::Program` object returned by .
On 2016/11/22 15:51:17, Kevin Millikin (Google) wrote:
> Missing the name of the function that returns the program.

Done.

https://codereview.chromium.org/2525623002/diff/1/runtime/include/dart_api.h#...
runtime/include/dart_api.h:2856: * Lonstructs a kernel program form a binary.
On 2016/11/22 15:51:17, Kevin Millikin (Google) wrote:
> Lonstructs == Load + constructs?

Precisely!

Done

https://codereview.chromium.org/2525623002/diff/1/runtime/include/dart_api.h#...
runtime/include/dart_api.h:2863: DART_EXPORT void* Dart_ParseKernelBinary(const
uint8_t* buffer,
On 2016/11/22 15:51:17, Kevin Millikin (Google) wrote:
> I don't think of this as parsing.  Deserializing, but maybe that's too long. 
> "ReadKernelBinary"?  "LoadKernelBinary"?

Then I'll go with ReadKernelBinary ("load" is already used for loading libraries
from the kernel program into the isolate)

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

https://codereview.chromium.org/2525623002/diff/1/runtime/vm/dart.cc#newcode515
runtime/vm/dart.cc:515: if (kernel_program != NULL) {
On 2016/11/22 15:51:17, Kevin Millikin (Google) wrote:
> Here you can just unconditionally call Object::Init(I, kernel_program).

Done.

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

https://codereview.chromium.org/2525623002/diff/1/runtime/vm/dart_api_impl.cc...
runtime/vm/dart_api_impl.cc:5387: // free it because (similar to the token
stream) it will be used to run our
On 2016/11/22 15:51:17, Kevin Millikin (Google) wrote:
> "used to repeatedly run the `kernel::FlowGraphBuilder`."

Done.