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

Issue 9017009: Reduce signal sender thread stack size to 32k.

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

Description

Reduce signal sender thread stack size to 32k. Commit partial old-space pages to reduce minimum memory use.

Patch Set 1 #

Total comments: 14

Patch Set 2 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+511 lines, -291 lines) Patch
M src/cpu-profiler.cc View 1 1 chunk +2 lines, -1 line 0 comments Download
M src/d8.cc View 1 2 chunks +5 lines, -5 lines 0 comments Download
M src/deoptimizer.cc View 1 1 chunk +1 line, -0 lines 0 comments Download
M src/heap.cc View 1 8 chunks +27 lines, -7 lines 0 comments Download
M src/incremental-marking.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M src/mark-compact.cc View 1 9 chunks +16 lines, -13 lines 0 comments Download
M src/platform.h View 1 1 chunk +11 lines, -5 lines 0 comments Download
M src/platform-freebsd.cc View 1 3 chunks +6 lines, -11 lines 0 comments Download
M src/platform-linux.cc View 1 3 chunks +6 lines, -11 lines 0 comments Download
M src/platform-macos.cc View 1 4 chunks +9 lines, -12 lines 0 comments Download
M src/platform-openbsd.cc View 1 3 chunks +6 lines, -11 lines 0 comments Download
M src/platform-solaris.cc View 1 3 chunks +7 lines, -11 lines 0 comments Download
M src/platform-win32.cc View 1 4 chunks +7 lines, -12 lines 0 comments Download
M src/serialize.cc View 1 2 chunks +7 lines, -1 line 0 comments Download
M src/snapshot.h View 1 2 chunks +16 lines, -0 lines 0 comments Download
M src/spaces.h View 1 24 chunks +57 lines, -46 lines 0 comments Download
M src/spaces.cc View 1 31 chunks +256 lines, -87 lines 0 comments Download
M src/spaces-inl.h View 1 3 chunks +7 lines, -6 lines 0 comments Download
M src/store-buffer.cc View 1 2 chunks +40 lines, -36 lines 0 comments Download
M src/utils.h View 1 2 chunks +4 lines, -6 lines 0 comments Download
M test/cctest/test-heap.cc View 1 1 chunk +2 lines, -5 lines 0 comments Download
M test/cctest/test-mark-compact.cc View 1 1 chunk +14 lines, -1 line 0 comments Download
M test/cctest/test-spaces.cc View 1 2 chunks +4 lines, -3 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
Erik Corry
9 years ago (2011-12-21 10:20:12 UTC) #1
Vyacheslav Egorov (Chromium)
9 years ago (2011-12-21 13:22:12 UTC) #2
http://codereview.chromium.org/9017009/diff/1/src/d8.cc
File src/d8.cc (right):

http://codereview.chromium.org/9017009/diff/1/src/d8.cc#newcode1087
src/d8.cc:1087: return i::Thread::Options("IsolateThread", 2 << 20);
I would prefer 2 * MB instead of 2 << 20.

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

http://codereview.chromium.org/9017009/diff/1/src/heap.cc#newcode629
src/heap.cc:629: if (gc_performed) one_gc_has_been_performed = true;
scavenge will not free any space in LO.

gc_performed does not imply full_gc_performed.

http://codereview.chromium.org/9017009/diff/1/src/heap.cc#newcode5312
src/heap.cc:5312: max_semispace_size_ =
SignedRoundUpToPowerOf2(max_semispace_size_);
what's the point of these fields being signed? looks ugly.

http://codereview.chromium.org/9017009/diff/1/src/platform-macos.cc
File src/platform-macos.cc (right):

http://codereview.chromium.org/9017009/diff/1/src/platform-macos.cc#newcode734
src/platform-macos.cc:734: static const int kSamplerThreadStackSize = 32 * KB;
sometimes you put constants into the class, sometimes outside of it.
consistency?

http://codereview.chromium.org/9017009/diff/1/src/spaces.cc
File src/spaces.cc (right):

http://codereview.chromium.org/9017009/diff/1/src/spaces.cc#newcode383
src/spaces.cc:383: intptr_t expand = Min(Max(size(), space_needed), kPageSize -
size());
add a comment that you are trying to a least double it in size.

http://codereview.chromium.org/9017009/diff/1/src/spaces.cc#newcode412
src/spaces.cc:412: paged_space->AddToFreeLists(old_end, new_area - old_end);
it looks a bit spooky it seems we will be wasting region from end_int - end_int
% paged_space->ObjectAlignment() to aligned_end_int until next big GC.

it's not much, but probably should be noted in comments.

http://codereview.chromium.org/9017009/diff/1/src/spaces.cc#newcode509
src/spaces.cc:509: size_t chunk_size =
Max(static_cast<intptr_t>(Page::kPageSize),
why are we wasting address space? if body size is smaller than kObjectAreaSize
(which might happen if we want to allocate some small immovable object).

http://codereview.chromium.org/9017009/diff/1/src/spaces.cc#newcode637
src/spaces.cc:637: RoundUp(chunk->size(), Page::kPageSize),
I don't like disagreement between chunk size and actually committed memory size.

http://codereview.chromium.org/9017009/diff/1/src/spaces.cc#newcode809
src/spaces.cc:809: if (last_page->size() < Page::kPageSize &&
comment is talking about "current page" but the code deals with the last page.

http://codereview.chromium.org/9017009/diff/1/src/spaces.cc#newcode823
src/spaces.cc:823: if (identity() == OLD_POINTER_SPACE) initial = initial * 8;
This looks very magical. I highly recommend to propagate information about the
requester with specific needs (e.g. by introducing
ReserveLinearAllocationRegion) to here and make all magic conditional.

http://codereview.chromium.org/9017009/diff/1/src/spaces.cc#newcode833
src/spaces.cc:833: SignedRoundUpToPowerOf2(
SignedRoundUpToPowerOf2 does not look right. Should we finally make all positive
size values unsigned?

http://codereview.chromium.org/9017009/diff/1/src/spaces.cc#newcode1917
src/spaces.cc:1917: if (large_list_ != NULL &&
The same piece of (complicated) code is repeated two times. Can you factor it
out into a separate function?

http://codereview.chromium.org/9017009/diff/1/src/spaces.cc#newcode2264
src/spaces.cc:2264: // The AddToFreeLists call above will reduce the size of the
space in the
Allocate is increasing size in the accounting stats. Who is going to do that
instead of it?

http://codereview.chromium.org/9017009/diff/1/src/utils.h
File src/utils.h (right):

http://codereview.chromium.org/9017009/diff/1/src/utils.h#newcode175
src/utils.h:175: uintptr_t x = static_cast<uintptr_t>(x_argument);
inline int SignedRoundUpToPowerOf2(int_type x_argument) {
  return static_cast<int>(RoundUpToPowerOf2(static_cast<uintptr_t>(x)));
}

Powered by Google App Engine
This is Rietveld 408576698