Reduce number of times a deopt-id is computed.

Reduce number of times a deopt-id is computed.

R=iposva@google.com

Committed: https://code.google.com/p/dart/source/detail?r=40716

[srdjan, 2014-09-22 23:21:53]

srdjan@google.com changed reviewers:
+ iposva@google.com

[srdjan, 2014-09-22 23:21:54]

This is a less invasive CL that reduces the number of changes while reducing the
number of calls to Isolate::Current(). In a next CL we could pass isolate for,
e.g., instance and static calls.

[Florian Schneider, 2014-09-25 09:55:34]

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

[Florian Schneider, 2014-09-25 09:55:35]

https://codereview.chromium.org/595673002/diff/20001/runtime/vm/intermediate_...
File runtime/vm/intermediate_language.h (right):

https://codereview.chromium.org/595673002/diff/20001/runtime/vm/intermediate_...
runtime/vm/intermediate_language.h:927: // deopt_id_ access.
How about moving deopt_id_ out of Instruction and only have it in those
instructions that need it (essentially the list of friend classes here and
above)

This way we can also get rid of these friend class declarations and save space
in unoptimzied instructions that never need a deopt-id (e.g. LoadLocal,
PushArgument, StoreLocal, ConstantIntr, etc)

[srdjan, 2014-09-25 17:42:16]

https://codereview.chromium.org/595673002/diff/20001/runtime/vm/intermediate_...
File runtime/vm/intermediate_language.h (right):

https://codereview.chromium.org/595673002/diff/20001/runtime/vm/intermediate_...
runtime/vm/intermediate_language.h:927: // deopt_id_ access.
On 2014/09/25 09:55:34, Florian Schneider wrote:
> How about moving deopt_id_ out of Instruction and only have it in those
> instructions that need it (essentially the list of friend classes here and
> above)
> 
> This way we can also get rid of these friend class declarations and save space
> in unoptimzied instructions that never need a deopt-id (e.g. LoadLocal,
> PushArgument, StoreLocal, ConstantIntr, etc)

That is a much more invasive change, but should reduce compiler's memory
consumption. I will try it out.

[srdjan, 2014-09-25 23:01:16]

On 2014/09/25 17:42:16, srdjan wrote:
>
https://codereview.chromium.org/595673002/diff/20001/runtime/vm/intermediate_...
> File runtime/vm/intermediate_language.h (right):
> 
>
https://codereview.chromium.org/595673002/diff/20001/runtime/vm/intermediate_...
> runtime/vm/intermediate_language.h:927: // deopt_id_ access.
> On 2014/09/25 09:55:34, Florian Schneider wrote:
> > How about moving deopt_id_ out of Instruction and only have it in those
> > instructions that need it (essentially the list of friend classes here and
> > above)
> > 
> > This way we can also get rid of these friend class declarations and save
space
> > in unoptimzied instructions that never need a deopt-id (e.g. LoadLocal,
> > PushArgument, StoreLocal, ConstantIntr, etc)
> 
> That is a much more invasive change, but should reduce compiler's memory
> consumption. I will try it out.

Tried mving deopt_id to individual instruction and the CL is too complicated: it
requires every instruction to have a field and a getter and a setter defined.
This CL is a change simple enough and has the expected performance benefits.

[Ivan Posva, 2014-09-26 16:27:09]

LGTM for now.

Longer term we should investigate ways which use specific constructors for the
deopt_id as we have discussed.

-Ivan

https://codereview.chromium.org/595673002/diff/60001/runtime/vm/code_descript...
File runtime/vm/code_descriptors.cc (right):

https://codereview.chromium.org/595673002/diff/60001/runtime/vm/code_descript...
runtime/vm/code_descriptors.cc:16: (deopt_id != Isolate::kNoDeoptId));
Separate CL: Move kNoDeoptId somewhere more sensible.

[srdjan, 2014-09-26 16:42:57]

Committed patchset #5 (id:80001) manually as r40716 (presubmit successful).