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

Issue 7621014: Fix the thresholds so that the heap does not grow uncontrollably. This fixes (Closed)

Created:
9 years, 4 months ago by Erik Corry
Modified:
9 years, 4 months ago
CC:
v8-dev
Visibility:
Public.

Description

Fix the thresholds so that the heap does not grow uncontrollably. This fixes the OOM on the 524 test, alllowing the underlying crash issue to become visible again. Committed: http://code.google.com/p/v8/source/detail?r=8918

Patch Set 1 #

Total comments: 9

Patch Set 2 : '' #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats (+66 lines, -20 lines) Patch
M src/heap.h View 1 2 chunks +39 lines, -3 lines 2 comments Download
M src/heap.cc View 1 5 chunks +12 lines, -17 lines 2 comments Download
M src/spaces.h View 1 2 chunks +6 lines, -0 lines 0 comments Download
M src/spaces.cc View 1 chunk +9 lines, -0 lines 2 comments Download

Messages

Total messages: 5 (0 generated)
Erik Corry
9 years, 4 months ago (2011-08-11 14:45:17 UTC) #1
Vyacheslav Egorov (Chromium)
LGTM with comments. http://codereview.chromium.org/7621014/diff/1/src/heap.cc File src/heap.cc (right): http://codereview.chromium.org/7621014/diff/1/src/heap.cc#newcode733 src/heap.cc:733: new_space()->Capacity() + AllocationFactor(old_gen_size)); maybe instead of ...
9 years, 4 months ago (2011-08-11 15:11:34 UTC) #2
Erik Corry
I rewrote it because it was so confusing. http://codereview.chromium.org/7621014/diff/1/src/heap.cc File src/heap.cc (right): http://codereview.chromium.org/7621014/diff/1/src/heap.cc#newcode733 src/heap.cc:733: new_space()->Capacity() ...
9 years, 4 months ago (2011-08-11 16:09:08 UTC) #3
Vyacheslav Egorov (Chromium)
LGTM! http://codereview.chromium.org/7621014/diff/5001/src/heap.cc File src/heap.cc (right): http://codereview.chromium.org/7621014/diff/5001/src/heap.cc#newcode738 src/heap.cc:738: old_gen_limit_factor_ = 1; I think you wanted 2 ...
9 years, 4 months ago (2011-08-12 09:11:51 UTC) #4
Erik Corry
9 years, 4 months ago (2011-08-12 09:21:47 UTC) #5
http://codereview.chromium.org/7621014/diff/5001/src/heap.cc
File src/heap.cc (right):

http://codereview.chromium.org/7621014/diff/5001/src/heap.cc#newcode738
src/heap.cc:738: old_gen_limit_factor_ = 1;
On 2011/08/12 09:11:51, Vyacheslav Egorov wrote:
> I think you wanted 2 here.

Good point.

http://codereview.chromium.org/7621014/diff/5001/src/heap.h
File src/heap.h (right):

http://codereview.chromium.org/7621014/diff/5001/src/heap.h#newcode1233
src/heap.h:1233: limit += new_space_.Capacity();
On 2011/08/12 09:11:51, Vyacheslav Egorov wrote:
> new_space_.Capacity can change while lazy sweeping is in progress. 
> 
> So Lower... will fluctuate. Is it intentional?

No, but I don't think it matters.

http://codereview.chromium.org/7621014/diff/5001/src/spaces.cc
File src/spaces.cc (right):

http://codereview.chromium.org/7621014/diff/5001/src/spaces.cc#newcode1933
src/spaces.cc:1933: if (FLAG_trace_gc && first_unswept_page_ != NULL) {
On 2011/08/12 09:11:51, Vyacheslav Egorov wrote:
> _verbose

Actually this happens rarely and if it happens it might be an issue to so I will
leave it in.

Powered by Google App Engine
This is Rietveld 408576698