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

Issue 2311963002: [heap,snapshot] Replace first page size from snapshots with page trimming (Closed)

Created:
4 years, 3 months ago by Michael Lippautz
Modified:
4 years, 3 months ago
Reviewers:
ulan, vogelheim
CC:
v8-reviews_googlegroups.com, Hannes Payer (out of office), ulan, Yang
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[heap,snapshot] Replace first page size from snapshots with page trimming Replace first page size in the snapshots with a heap logic that trims pages after deserialization. The snapshot provided page sizes was just an approximation, while the heap knows exactly where to trim. Furthermore, trim the pages directly after deserialization, leaving no wiggle room for further objects. This avoids pollution of the immortal immovable pages with regular objects, e.g. Contexts. The downside is that we potentially require expanding the space with a new page. BUG=chromium:636331 Committed: https://crrev.com/ed8791ea65ea1f37520a5ef71b3ae10150329ed3 Cr-Commit-Position: refs/heads/master@{#39200}

Patch Set 1 #

Patch Set 2 : Fix current tests and add shrinking tests #

Patch Set 3 : Copy over fixes for TestAlignedOverAllocation #

Patch Set 4 : Add print for debugging #

Patch Set 5 : Remove debugging print #

Unified diffs Side-by-side diffs Delta from patch set Stats (+271 lines, -108 lines) Patch
M src/heap/heap.cc View 1 chunk +7 lines, -4 lines 0 comments Download
M src/heap/spaces.h View 3 chunks +8 lines, -0 lines 0 comments Download
M src/heap/spaces.cc View 5 chunks +72 lines, -8 lines 0 comments Download
M src/isolate.cc View 2 chunks +13 lines, -9 lines 0 comments Download
M src/objects.h View 1 chunk +1 line, -0 lines 0 comments Download
M src/snapshot/snapshot.h View 2 chunks +4 lines, -12 lines 0 comments Download
M src/snapshot/snapshot-common.cc View 4 chunks +3 lines, -67 lines 0 comments Download
M test/cctest/heap/heap-utils.h View 1 1 chunk +6 lines, -0 lines 0 comments Download
M test/cctest/heap/heap-utils.cc View 1 1 chunk +29 lines, -0 lines 0 comments Download
M test/cctest/heap/test-heap.cc View 1 2 1 chunk +12 lines, -0 lines 0 comments Download
M test/cctest/heap/test-spaces.cc View 1 4 chunks +116 lines, -8 lines 0 comments Download

Messages

Total messages: 34 (28 generated)
Michael Lippautz
ptal: - Daniel: snapshot/ - Ulan: rest Originally reviewed here: https://codereview.chromium.org/2013713003. Basically, this is the ...
4 years, 3 months ago (2016-09-06 08:07:43 UTC) #16
ulan
lgtm
4 years, 3 months ago (2016-09-06 08:35:39 UTC) #22
vogelheim
lgtm for src/snapshot/*
4 years, 3 months ago (2016-09-06 08:47:55 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2311963002/100001
4 years, 3 months ago (2016-09-06 11:00:38 UTC) #30
commit-bot: I haz the power
Committed patchset #5 (id:100001)
4 years, 3 months ago (2016-09-06 11:02:25 UTC) #32
commit-bot: I haz the power
4 years, 3 months ago (2016-09-06 11:03:11 UTC) #34
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/ed8791ea65ea1f37520a5ef71b3ae10150329ed3
Cr-Commit-Position: refs/heads/master@{#39200}

Powered by Google App Engine
This is Rietveld 408576698