Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(650)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 9207002: Add a deoptimization count to each function to limit number of re-compilations. (Closed)

Created:
8 years, 11 months ago by fschneider
Modified:
7 years, 6 months ago
Reviewers:
Jakob Kummerow
CC:
v8-dev
Visibility:
Public.

Description

Add a deoptimization count to each function to limit number of re-compilations. In most scenarios it is better to count deoptimizations than optimizations to decide when to stop re-compiling a function because of repeated deopts. BUG=v8:1902

Patch Set 1 #

Total comments: 26
Unified diffs Side-by-side diffs Delta from patch set Stats (+77 lines, -70 lines) Patch
M src/arm/deoptimizer-arm.cc View 1 chunk +2 lines, -0 lines 2 comments Download
M src/compiler.h View 1 chunk +1 line, -1 line 2 comments Download
M src/compiler.cc View 4 chunks +2 lines, -19 lines 2 comments Download
M src/heap.cc View 2 chunks +2 lines, -2 lines 2 comments Download
M src/hydrogen.cc View 3 chunks +4 lines, -3 lines 0 comments Download
M src/ia32/deoptimizer-ia32.cc View 1 chunk +2 lines, -0 lines 2 comments Download
M src/mips/deoptimizer-mips.cc View 1 chunk +2 lines, -0 lines 2 comments Download
M src/objects.h View 7 chunks +17 lines, -15 lines 4 comments Download
M src/objects.cc View 4 chunks +19 lines, -10 lines 2 comments Download
M src/objects-inl.h View 3 chunks +4 lines, -4 lines 2 comments Download
M src/runtime.h View 1 chunk +1 line, -1 line 2 comments Download
M src/runtime.cc View 2 chunks +5 lines, -2 lines 0 comments Download
M src/x64/deoptimizer-x64.cc View 1 chunk +2 lines, -0 lines 2 comments Download
M test/mjsunit/assert-opt-and-deopt.js View 4 chunks +14 lines, -13 lines 2 comments Download

Messages

Total messages: 3 (0 generated)
fschneider
8 years, 11 months ago (2012-01-13 15:12:06 UTC) #1
Jakob Kummerow
LGTM with a bunch of nits. http://codereview.chromium.org/9207002/diff/1/src/arm/deoptimizer-arm.cc File src/arm/deoptimizer-arm.cc (right): http://codereview.chromium.org/9207002/diff/1/src/arm/deoptimizer-arm.cc#newcode1 src/arm/deoptimizer-arm.cc:1: // Copyright 2011 ...
8 years, 11 months ago (2012-01-16 11:41:25 UTC) #2
fschneider
8 years, 11 months ago (2012-01-19 10:26:11 UTC) #3 
Thanks for reviewing. I still need to fix some performance issues before this
can be landed.

http://codereview.chromium.org/9207002/diff/1/src/arm/deoptimizer-arm.cc
File src/arm/deoptimizer-arm.cc (right):

http://codereview.chromium.org/9207002/diff/1/src/arm/deoptimizer-arm.cc#newc...
src/arm/deoptimizer-arm.cc:1: // Copyright 2011 the V8 project authors. All
rights reserved.
On 2012/01/16 11:41:25, Jakob wrote:
> nit: 2012

Done.

http://codereview.chromium.org/9207002/diff/1/src/compiler.cc
File src/compiler.cc (right):

http://codereview.chromium.org/9207002/diff/1/src/compiler.cc#newcode1
src/compiler.cc:1: // Copyright 2011 the V8 project authors. All rights
reserved.
On 2012/01/16 11:41:25, Jakob wrote:
> nit: 2012

Done.

http://codereview.chromium.org/9207002/diff/1/src/compiler.h
File src/compiler.h (right):

http://codereview.chromium.org/9207002/diff/1/src/compiler.h#newcode1
src/compiler.h:1: // Copyright 2011 the V8 project authors. All rights reserved.
On 2012/01/16 11:41:25, Jakob wrote:
> nit: 2012

Done.

http://codereview.chromium.org/9207002/diff/1/src/heap.cc
File src/heap.cc (right):

http://codereview.chromium.org/9207002/diff/1/src/heap.cc#newcode1
src/heap.cc:1: // Copyright 2011 the V8 project authors. All rights reserved.
On 2012/01/16 11:41:25, Jakob wrote:
> nit: 2012

Done.

http://codereview.chromium.org/9207002/diff/1/src/ia32/deoptimizer-ia32.cc
File src/ia32/deoptimizer-ia32.cc (right):

http://codereview.chromium.org/9207002/diff/1/src/ia32/deoptimizer-ia32.cc#ne...
src/ia32/deoptimizer-ia32.cc:1: // Copyright 2011 the V8 project authors. All
rights reserved.
On 2012/01/16 11:41:25, Jakob wrote:
> nit: 2012

Done.

http://codereview.chromium.org/9207002/diff/1/src/mips/deoptimizer-mips.cc
File src/mips/deoptimizer-mips.cc (right):

http://codereview.chromium.org/9207002/diff/1/src/mips/deoptimizer-mips.cc#ne...
src/mips/deoptimizer-mips.cc:1: // Copyright 2011 the V8 project authors. All
rights reserved.
On 2012/01/16 11:41:25, Jakob wrote:
> nit: 2012

Done.

http://codereview.chromium.org/9207002/diff/1/src/objects-inl.h
File src/objects-inl.h (right):

http://codereview.chromium.org/9207002/diff/1/src/objects-inl.h#newcode1
src/objects-inl.h:1: // Copyright 2011 the V8 project authors. All rights
reserved.
On 2012/01/16 11:41:25, Jakob wrote:
> nit: 2012

Done.

http://codereview.chromium.org/9207002/diff/1/src/objects.cc
File src/objects.cc (right):

http://codereview.chromium.org/9207002/diff/1/src/objects.cc#newcode8020
src/objects.cc:8020: function->shared()->PrintName(out);
On 2012/01/16 11:41:25, Jakob wrote:
> nit: since you defined JSFunction::PrintName(...), this change seems
> unnecessary, but I don't mind if you leave it in.

Done.

http://codereview.chromium.org/9207002/diff/1/src/objects.h
File src/objects.h (right):

http://codereview.chromium.org/9207002/diff/1/src/objects.h#newcode5195
src/objects.h:5195: // Disable (further) attempted optimization of all functions
sharing this
On 2012/01/16 11:41:25, Jakob wrote:
> nit: truncated comment (missing "shared function info.")

Done.

http://codereview.chromium.org/9207002/diff/1/src/objects.h#newcode5286
src/objects.h:5286: static const int kDeoptCounterOffset =
On 2012/01/16 11:41:25, Jakob wrote:
> Rename this to kStressDeoptCounterOffset?

Done.

http://codereview.chromium.org/9207002/diff/1/src/runtime.h
File src/runtime.h (right):

http://codereview.chromium.org/9207002/diff/1/src/runtime.h#newcode1
src/runtime.h:1: // Copyright 2011 the V8 project authors. All rights reserved.
On 2012/01/16 11:41:25, Jakob wrote:
> nit: 2012

Done.

http://codereview.chromium.org/9207002/diff/1/src/x64/deoptimizer-x64.cc
File src/x64/deoptimizer-x64.cc (right):

http://codereview.chromium.org/9207002/diff/1/src/x64/deoptimizer-x64.cc#newc...
src/x64/deoptimizer-x64.cc:1: // Copyright 2011 the V8 project authors. All
rights reserved.
On 2012/01/16 11:41:25, Jakob wrote:
> nit: 2012

Done.

http://codereview.chromium.org/9207002/diff/1/test/mjsunit/assert-opt-and-deo...
File test/mjsunit/assert-opt-and-deopt.js (right):

http://codereview.chromium.org/9207002/diff/1/test/mjsunit/assert-opt-and-deo...
test/mjsunit/assert-opt-and-deopt.js:1: // Copyright 2011 the V8 project
authors. All rights reserved.
On 2012/01/16 11:41:25, Jakob wrote:
> nit: 2012

Done.

Powered by Google App Engine
This is Rietveld 408576698