Deoptimize functions before setting breakpoints

Deoptimize functions before setting breakpoints

Does not yet handle cases where the functions have active frames
on the stack.
Committed: https://code.google.com/p/dart/source/detail?r=5436

[srdjan, 2012-03-13 17:11:35]

LGTM with comments

http://codereview.chromium.org/9696020/diff/3001/runtime/vm/code_generator.cc
File runtime/vm/code_generator.cc (right):

http://codereview.chromium.org/9696020/diff/3001/runtime/vm/code_generator.cc...
runtime/vm/code_generator.cc:1171: if (isolate->debugger()->IsActive()) {
Maybe add comment why we should not optimize when debugger is active.

http://codereview.chromium.org/9696020/diff/3001/runtime/vm/debugger.cc
File runtime/vm/debugger.cc (right):

http://codereview.chromium.org/9696020/diff/3001/runtime/vm/debugger.cc#newco...
runtime/vm/debugger.cc:25: static const bool verbose = false;
Why not add a debugger_verbose flag?

http://codereview.chromium.org/9696020/diff/3001/runtime/vm/debugger.cc#newco...
runtime/vm/debugger.cc:494: printf("Deoptimizing function %s\n",
OS::Print

http://codereview.chromium.org/9696020/diff/3001/runtime/vm/debugger.cc#newco...
runtime/vm/debugger.cc:498:
func.set_deoptimization_counter(func.deoptimization_counter() + 1);
Don't increase the counter otherwise setting and clearing the breakpoint would
prevent future optimizations. The deopt counter is here to stop endless
deoptimizations of certain bad-behaving methods,

http://codereview.chromium.org/9696020/diff/3001/runtime/vm/debugger.cc#newco...
runtime/vm/debugger.cc:514: EnsureFunctionIsDeoptimized(target_function);
I rather think positive:

if (target.HasCode()) {
  EnsureFunctionIsDeoptimized(target_function);
} else {
  ...
}

[hausner, 2012-03-13 20:45:03]

http://codereview.chromium.org/9696020/diff/3001/runtime/vm/code_generator.cc
File runtime/vm/code_generator.cc (right):

http://codereview.chromium.org/9696020/diff/3001/runtime/vm/code_generator.cc...
runtime/vm/code_generator.cc:1171: if (isolate->debugger()->IsActive()) {
On 2012/03/13 17:11:35, srdjan wrote:
> Maybe add comment why we should not optimize when debugger is active.

Done.

http://codereview.chromium.org/9696020/diff/3001/runtime/vm/debugger.cc
File runtime/vm/debugger.cc (right):

http://codereview.chromium.org/9696020/diff/3001/runtime/vm/debugger.cc#newco...
runtime/vm/debugger.cc:25: static const bool verbose = false;
On 2012/03/13 17:11:35, srdjan wrote:
> Why not add a debugger_verbose flag?
Soon I'll give in and do so. So for I intend to remove this flag once the
debugger is fairly stable.

http://codereview.chromium.org/9696020/diff/3001/runtime/vm/debugger.cc#newco...
runtime/vm/debugger.cc:494: printf("Deoptimizing function %s\n",
On 2012/03/13 17:11:35, srdjan wrote:
> OS::Print

Done.

http://codereview.chromium.org/9696020/diff/3001/runtime/vm/debugger.cc#newco...
runtime/vm/debugger.cc:498:
func.set_deoptimization_counter(func.deoptimization_counter() + 1);
On 2012/03/13 17:11:35, srdjan wrote:
> Don't increase the counter otherwise setting and clearing the breakpoint would
> prevent future optimizations. The deopt counter is here to stop endless
> deoptimizations of certain bad-behaving methods,

Done.

http://codereview.chromium.org/9696020/diff/3001/runtime/vm/debugger.cc#newco...
runtime/vm/debugger.cc:514: EnsureFunctionIsDeoptimized(target_function);
The glass is now half full.

On 2012/03/13 17:11:35, srdjan wrote:
> I rather think positive:
> 
> if (target.HasCode()) {
>   EnsureFunctionIsDeoptimized(target_function);
> } else {
>   ...
> }