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

Issue 11275145: Decouple allocation and creation of deopt tables (Closed)

Created:
8 years, 1 month ago by danno
Modified:
8 years, 1 month ago
Reviewers:
Jakob Kummerow
CC:
v8-dev
Visibility:
Public.

Description

Decouple allocation and creation of deopt tables This makes it possible to calculate the future address of a deopt entry before it is possible to generate the deopt table. Committed: https://code.google.com/p/v8/source/detail?r=12877

Patch Set 1 #

Patch Set 2 : Grow deopt tables on demand #

Patch Set 3 : Revert MIPS support #

Patch Set 4 : nits #

Patch Set 5 : Ready for review #

Total comments: 5

Patch Set 6 : Review feedback #

Unified diffs Side-by-side diffs Delta from patch set Stats (+87 lines, -45 lines) Patch
M src/api.cc View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M src/deoptimizer.h View 1 2 3 4 4 chunks +21 lines, -5 lines 0 comments Download
M src/deoptimizer.cc View 1 2 3 4 5 5 chunks +65 lines, -40 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
danno
PTAL
8 years, 1 month ago (2012-11-06 09:00:37 UTC) #1
Jakob Kummerow
LGTM with nits. http://codereview.chromium.org/11275145/diff/8001/src/deoptimizer.cc File src/deoptimizer.cc (right): http://codereview.chromium.org/11275145/diff/8001/src/deoptimizer.cc#newcode58 src/deoptimizer.cc:58: if (eager_deoptimization_entry_code_ != NULL) { nit: ...
8 years, 1 month ago (2012-11-06 11:40:49 UTC) #2
danno
8 years, 1 month ago (2012-11-06 11:59:03 UTC) #3
Feedback addressed, I will land this after verifying there's not performance
regression.

http://codereview.chromium.org/11275145/diff/8001/src/deoptimizer.cc
File src/deoptimizer.cc (right):

http://codereview.chromium.org/11275145/diff/8001/src/deoptimizer.cc#newcode58
src/deoptimizer.cc:58: if (eager_deoptimization_entry_code_ != NULL) {
On 2012/11/06 11:40:50, Jakob wrote:
> nit: you can simplify this a bit by removing the condition. |delete| is safe
to
> call on NULL pointers.
> 
> Again below.

Done.

http://codereview.chromium.org/11275145/diff/8001/src/deoptimizer.cc#newcode478
src/deoptimizer.cc:478: Address Deoptimizer::GetDeoptimizationEntry(int id,
BailoutType type,
On 2012/11/06 11:40:50, Jakob wrote:
> nit: one line per argument

Done.

Powered by Google App Engine
This is Rietveld 408576698