VM: Make use_osr an Isolate flag, similar to how we made use_field_guards.

VM: Make use_osr an Isolate flag, similar to how we made use_field_guards.

In Debug mode we have assertions checking that we don't attempt OSR when
FLAG_use_osr is disabled - however app-jit snapshots are compiled with OSR
enabled and still contain countining and OSR attempting code, which causes
assertions to fail.

Refactor how getters for isolate flags are defined, consolidate all flags into
a single list.

Update test expectations and switch -c dartk configuration to use app-jit snapshot.

BUG=
R=rmacnak@google.com

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

[Vyacheslav Egorov (Google), 2017-03-03 16:17:03]

Description was changed from

==========
VM: Make use_osr an Isolate flag, similar to how we made use_field_guards.

In Debug mode we have assertions checking that we don't attempt OSR when
FLAG_use_osr is disabled - however app-jit snapshots are compiled with OSR
enabled and still contain countining and OSR attempting code, which causes
assertions to fail.

Refactor how getters for isolate flags are defined, consolidate all flags into
a single list.

BUG=
==========

to

==========
VM: Make use_osr an Isolate flag, similar to how we made use_field_guards.

In Debug mode we have assertions checking that we don't attempt OSR when
FLAG_use_osr is disabled - however app-jit snapshots are compiled with OSR
enabled and still contain countining and OSR attempting code, which causes
assertions to fail.

Refactor how getters for isolate flags are defined, consolidate all flags into
a single list.

Update test expectations and switch -c dartk configuration to use app-jit
snapshot.

BUG=
==========

[Vyacheslav Egorov (Google), 2017-03-03 16:17:04]

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

[Vyacheslav Egorov (Google), 2017-03-03 16:17:18]

Hi Martin & Ryan! Please take a look.

[Florian Schneider, 2017-03-03 16:59:50]

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

[Florian Schneider, 2017-03-03 16:59:51]

It seems that now we have to potentially make a lot of flags isolate flags.
Wouldn't it be cleaner to check for the flag when attempting OSR instead of an
assertion failure? Or invalidate optimized snapshot code if the flags of the
snapshot training run don't match the flags at runtime?

[Vyacheslav Egorov (Google), 2017-03-03 17:08:24]

I don't anticipate as adding many more flags here (maybe things like heap
verification, stress flags etc), but in any case with the macro-list it's very
easy to add more. 

I think these flags should have been isolate specific to begin with, instead of
being global (they are just global for convenience).

For example if I switch OSR off for testing purposes, it's not clear why this
should affect Kernel Isolate or any other isolate except main. 

Invalidating code might be ok solution for OSR in particular, but that does not
really solve issues which are not code specific and also completely removes any
benefits app-jit gives us. 

Right now we reject snapshots with mismatching feature string which allows us to
catch app-jit mismatch situations eagerly instead of getting sudden silent
performance drop.

[rmacnak, 2017-03-03 17:44:57]

On 2017/03/03 17:08:24, Vyacheslav Egorov (Google) wrote:
> I don't anticipate as adding many more flags here (maybe things like heap
> verification, stress flags etc), but in any case with the macro-list it's very
> easy to add more. 
> 
> I think these flags should have been isolate specific to begin with, instead
of
> being global (they are just global for convenience).
> 
> For example if I switch OSR off for testing purposes, it's not clear why this
> should affect Kernel Isolate or any other isolate except main. 
> 
> Invalidating code might be ok solution for OSR in particular, but that does
not
> really solve issues which are not code specific and also completely removes
any
> benefits app-jit gives us. 
> 
> Right now we reject snapshots with mismatching feature string which allows us
to
> catch app-jit mismatch situations eagerly instead of getting sudden silent
> performance drop.

lgtm

I agree that we shouldn't invalidate code on a mismatch. Hitting a performance
cliff and dropping compiled code you had to explicitly ask for undermines the
point of generating an app-jit snapshot instead of a script snapshot. Maybe we
should set the flags based on the snapshot features? Though that also could be
lead to confusion about why behavior doesn't match flags you passed.

For OSR in particular, we should consider disabling it during training. If we
expect the real work to match the training, then a method that doesn't get
optimized during training probably shouldn't bother updating a counter in the
real work.

[rmacnak, 2017-03-03 17:45:15]

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

https://codereview.chromium.org/2728163002/diff/1/runtime/vm/dart.cc#newcode677
runtime/vm/dart.cc:677: ISOLATE_FLAG_LIST(ADD_FLAG);
The snapshot should still work even if FLAG_error_on_bad_type or
FLAG_error_on_bad_override change, but it is safer to just include all the
isolate flags.

[Vyacheslav Egorov (Google), 2017-03-03 18:02:28]

Description was changed from

==========
VM: Make use_osr an Isolate flag, similar to how we made use_field_guards.

In Debug mode we have assertions checking that we don't attempt OSR when
FLAG_use_osr is disabled - however app-jit snapshots are compiled with OSR
enabled and still contain countining and OSR attempting code, which causes
assertions to fail.

Refactor how getters for isolate flags are defined, consolidate all flags into
a single list.

Update test expectations and switch -c dartk configuration to use app-jit
snapshot.

BUG=
==========

to

==========
VM: Make use_osr an Isolate flag, similar to how we made use_field_guards.

In Debug mode we have assertions checking that we don't attempt OSR when
FLAG_use_osr is disabled - however app-jit snapshots are compiled with OSR
enabled and still contain countining and OSR attempting code, which causes
assertions to fail.

Refactor how getters for isolate flags are defined, consolidate all flags into
a single list.

Update test expectations and switch -c dartk configuration to use app-jit
snapshot.

BUG=
R=rmacnak@google.com

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

[Vyacheslav Egorov (Google), 2017-03-03 18:02:29]

Committed patchset #1 (id:1) manually as
021f933f6970dd6d62445f27c334b55df8233665 (presubmit successful).