- reorganize DartUtils::PrepareForScriptLoading so that it does not have the wait for service load …

- reorganize DartUtils::PrepareForScriptLoading so that it does not have the wait for service load port code in it. This is needed so that it is possible to not wait for the service load port when running with a full application snapshot that requires no loading.

- cache builtin_lib in IsolateData so that the cached value can be used withouit having to call Builtin::LoadAndCheckLibrary which creates new string objects everytime it is called.

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

[siva, 2016-02-03 22:50:47]

asiva@google.com changed reviewers:
+ johnmccutchan@google.com, zra@google.com

[Cutch, 2016-02-03 23:16:17]

LGTMwC

https://codereview.chromium.org/1663963002/diff/20001/runtime/vm/unit_test.h
File runtime/vm/unit_test.h (right):

https://codereview.chromium.org/1663963002/diff/20001/runtime/vm/unit_test.h#...
runtime/vm/unit_test.h:10: #include "bin/isolate_data.h"
This feels icky but I see that unit_test.cc already #includes things from bin/

[siva, 2016-02-03 23:52:04]

https://codereview.chromium.org/1663963002/diff/20001/runtime/vm/unit_test.h
File runtime/vm/unit_test.h (right):

https://codereview.chromium.org/1663963002/diff/20001/runtime/vm/unit_test.h#...
runtime/vm/unit_test.h:10: #include "bin/isolate_data.h"
On 2016/02/03 23:16:17, Cutch wrote:
> This feels icky but I see that unit_test.cc already #includes things from bin/

I have moved the functions to the '.cc' files so that we don't have to add the
includes here. The '.cc' files already have bin

[zra, 2016-02-04 04:12:41]

https://codereview.chromium.org/1663963002/diff/40001/runtime/bin/dartutils.cc
File runtime/bin/dartutils.cc (right):

https://codereview.chromium.org/1663963002/diff/40001/runtime/bin/dartutils.c...
runtime/bin/dartutils.cc:309: IsolateData* isolate_data =
This pattern is repeated a lot. Maybe something like this would useful:

Dart_Handle DartUtils::BuiltinLib() {
  IsolateData* isolate_data =
    reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
  return isolate_data->builtin_lib();
}

https://codereview.chromium.org/1663963002/diff/40001/runtime/bin/dartutils.c...
runtime/bin/dartutils.cc:312: Dart_Handle directory =
NewString(original_working_directory);
RETURN_IF_ERROR(directory)

Sometimes we check the result of NewString to see if it's an error and sometimes
we don't. I suspect that checking all of them is the safer bet so that we don't
end up going back into Dart code after we could have detected e.g. an OOM
exception.

Maybe not in this CL, though.

[siva, 2016-02-04 16:59:01]

https://codereview.chromium.org/1663963002/diff/40001/runtime/bin/dartutils.cc
File runtime/bin/dartutils.cc (right):

https://codereview.chromium.org/1663963002/diff/40001/runtime/bin/dartutils.c...
runtime/bin/dartutils.cc:309: IsolateData* isolate_data =
On 2016/02/04 04:12:41, zra wrote:
> This pattern is repeated a lot. Maybe something like this would useful:
> 
> Dart_Handle DartUtils::BuiltinLib() {
>   IsolateData* isolate_data =
>     reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
>   return isolate_data->builtin_lib();
> }

Done.

https://codereview.chromium.org/1663963002/diff/40001/runtime/bin/dartutils.c...
runtime/bin/dartutils.cc:312: Dart_Handle directory =
NewString(original_working_directory);
On 2016/02/04 04:12:41, zra wrote:
> RETURN_IF_ERROR(directory)
> 
> Sometimes we check the result of NewString to see if it's an error and
sometimes
> we don't. I suspect that checking all of them is the safer bet so that we
don't
> end up going back into Dart code after we could have detected e.g. an OOM
> exception.
> 
> Maybe not in this CL, though.

NewString is used in a number of places sometimes inline as a parameter, it is
going to require a lot of changes. Maybe another CL or a cleanup when time
permits.

[siva, 2016-02-04 17:18:34]

Description was changed from

==========
- reorganize DartUtils::PrepareForScriptLoading so that it does not have the
wait for service load port code in it. This is needed so that it is possible to
not wait for the service load port when running with a full application snapshot
that requires no loading.

- cache builtin_lib in IsolateData so that the cached value can be used withouit
having to call Builtin::LoadAndCheckLibrary which creates new string objects
everytime it is called.
==========

to

==========
- reorganize DartUtils::PrepareForScriptLoading so that it does not have the
wait for service load port code in it. This is needed so that it is possible to
not wait for the service load port when running with a full application snapshot
that requires no loading.

- cache builtin_lib in IsolateData so that the cached value can be used withouit
having to call Builtin::LoadAndCheckLibrary which creates new string objects
everytime it is called.

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

[siva, 2016-02-04 17:18:35]

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