Fixed resizing of deopt table

Fixed resizing of deopt table

BUG=chrome:166554
Committed: http://code.google.com/p/v8/source/detail?r=13237

[danno, 2012-12-18 15:49:55]

lgtm

[palmer, 2012-12-18 20:33:41]

https://codereview.chromium.org/11617018/diff/1/src/deoptimizer.cc
File src/deoptimizer.cc (right):

https://codereview.chromium.org/11617018/diff/1/src/deoptimizer.cc#newcode1477
src/deoptimizer.cc:1477: int max_entry_id) {
max_entry_id should be unsigned int (or, better, size_t), right?

https://codereview.chromium.org/11617018/diff/1/src/deoptimizer.cc#newcode1491
src/deoptimizer.cc:1491: while (max_entry_id >= entry_count) entry_count *= 2;
Risk of integer overflow? And why not use unsigned int here? (size_t is best)

[Sven Panne, 2012-12-19 07:34:58]

https://codereview.chromium.org/11617018/diff/1/src/deoptimizer.cc
File src/deoptimizer.cc (right):

https://codereview.chromium.org/11617018/diff/1/src/deoptimizer.cc#newcode1477
src/deoptimizer.cc:1477: int max_entry_id) {
On 2012/12/18 20:33:41, Chris P. wrote:
> max_entry_id should be unsigned int (or, better, size_t), right?

To be exact, neither of them: What we should really use here is a totally
separate type "DeoptimizationIndex", just like BailoutId or TypeFeedbackId,
which wraps an underlying integral type (for Haskellers: Use a newtype). If the
underlying integral type for this int, size_t or whatever doesn't really matter
that much, it depends on e.g. the fact how to handle "Maybe DeoptimizationIndex"
in Haskell-speak.

This might be a worthwhile refactoring, but should be done in a separate CL. It
is a real, real pity that we don't use such wrapped integral types much more
often, that would have avoided tons of portability and maintenance problems and
would be extremely beneficial for code readability.

https://codereview.chromium.org/11617018/diff/1/src/deoptimizer.cc#newcode1491
src/deoptimizer.cc:1491: while (max_entry_id >= entry_count) entry_count *= 2;
On 2012/12/18 20:33:41, Chris P. wrote:
> Risk of integer overflow? And why not use unsigned int here? (size_t is best)

Regarding typing: See above. Regarding overflow: Can't happen, see call sites,
perhaps another assertion might make this clearer, but I'll leave it for now as
it is.

[palmer, 2012-12-19 18:46:01]

> Regarding overflow: Can't happen, see call sites,
> perhaps another assertion might make this clearer, but I'll leave it for now
as
> it is.

Yeah, in general it's not great for callees to have to rely on their callers to
be trustworthy. It also makes code review harder. :) It'd be great to put an
assert in there.