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

Issue 1278413002: [interpreter] Fix nosnap build for interpreter table generation. (Closed)

Created:
5 years, 4 months ago by rmcilroy
Modified:
5 years, 4 months ago
CC:
v8-dev
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[interpreter] Fix nosnap build for interpreter table generation. Moves the creation of the interpreter table early on during initialization to ensure that even on nosnap builds it still gets allocated in the first page. BUG=v8:4280 LOG=N Committed: https://crrev.com/cc74437ba70633fcee817a45aca74b41fec37d38 Cr-Commit-Position: refs/heads/master@{#30096}

Patch Set 1 #

Patch Set 2 : Remove extra patchset #

Patch Set 3 : Ensure table generated where approprate #

Total comments: 2

Patch Set 4 : Remove initialized() #

Total comments: 4

Patch Set 5 : Remove create_heap_objects #

Unified diffs Side-by-side diffs Delta from patch set Stats (+54 lines, -34 lines) Patch
M src/heap/heap.h View 1 chunk +0 lines, -4 lines 0 comments Download
M src/heap/heap.cc View 3 chunks +4 lines, -2 lines 0 comments Download
M src/interpreter/interpreter.h View 1 2 3 4 2 chunks +9 lines, -1 line 0 comments Download
M src/interpreter/interpreter.cc View 1 2 3 4 1 chunk +38 lines, -19 lines 0 comments Download
M src/isolate.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M test/cctest/interpreter/test-interpreter.cc View 1 2 3 4 1 chunk +2 lines, -7 lines 0 comments Download

Messages

Total messages: 26 (9 generated)
rmcilroy
Hannes, could you have a quick look please (this is needed to green the tree). ...
5 years, 4 months ago (2015-08-10 14:13:37 UTC) #3
rmcilroy
Hannes, could you have a quick look please (this is needed to green the tree). ...
5 years, 4 months ago (2015-08-10 14:13:37 UTC) #4
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1278413002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1278413002/20001
5 years, 4 months ago (2015-08-10 14:14:06 UTC) #6
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 4 months ago (2015-08-10 14:39:59 UTC) #8
Hannes Payer (out of office)
LGTM
5 years, 4 months ago (2015-08-10 15:26:17 UTC) #9
Hannes Payer (out of office)
https://codereview.chromium.org/1278413002/diff/40001/src/interpreter/interpreter.cc File src/interpreter/interpreter.cc (right): https://codereview.chromium.org/1278413002/diff/40001/src/interpreter/interpreter.cc#newcode43 src/interpreter/interpreter.cc:43: if (initialized_) return; What is the purpose of initialized_?
5 years, 4 months ago (2015-08-10 16:28:05 UTC) #10
rmcilroy
https://codereview.chromium.org/1278413002/diff/40001/src/interpreter/interpreter.cc File src/interpreter/interpreter.cc (right): https://codereview.chromium.org/1278413002/diff/40001/src/interpreter/interpreter.cc#newcode43 src/interpreter/interpreter.cc:43: if (initialized_) return; On 2015/08/10 16:28:05, Hannes Payer wrote: ...
5 years, 4 months ago (2015-08-10 16:37:03 UTC) #11
Hannes Payer (out of office)
https://codereview.chromium.org/1278413002/diff/60001/src/interpreter/interpreter.cc File src/interpreter/interpreter.cc (right): https://codereview.chromium.org/1278413002/diff/60001/src/interpreter/interpreter.cc#newcode41 src/interpreter/interpreter.cc:41: void Interpreter::Initialize(bool create_heap_objects) { Why would you call this ...
5 years, 4 months ago (2015-08-10 16:50:00 UTC) #12
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1278413002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1278413002/60001
5 years, 4 months ago (2015-08-10 16:50:28 UTC) #14
rmcilroy
https://codereview.chromium.org/1278413002/diff/60001/src/interpreter/interpreter.cc File src/interpreter/interpreter.cc (right): https://codereview.chromium.org/1278413002/diff/60001/src/interpreter/interpreter.cc#newcode41 src/interpreter/interpreter.cc:41: void Interpreter::Initialize(bool create_heap_objects) { On 2015/08/10 16:50:00, Hannes Payer ...
5 years, 4 months ago (2015-08-10 16:54:36 UTC) #15
Hannes Payer (out of office)
https://codereview.chromium.org/1278413002/diff/60001/src/interpreter/interpreter.cc File src/interpreter/interpreter.cc (right): https://codereview.chromium.org/1278413002/diff/60001/src/interpreter/interpreter.cc#newcode41 src/interpreter/interpreter.cc:41: void Interpreter::Initialize(bool create_heap_objects) { On 2015/08/10 16:54:36, rmcilroy wrote: ...
5 years, 4 months ago (2015-08-10 16:59:26 UTC) #16
rmcilroy
https://codereview.chromium.org/1278413002/diff/60001/src/interpreter/interpreter.cc File src/interpreter/interpreter.cc (right): https://codereview.chromium.org/1278413002/diff/60001/src/interpreter/interpreter.cc#newcode41 src/interpreter/interpreter.cc:41: void Interpreter::Initialize(bool create_heap_objects) { On 2015/08/10 16:59:25, Hannes Payer ...
5 years, 4 months ago (2015-08-10 17:08:43 UTC) #17
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1278413002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1278413002/80001
5 years, 4 months ago (2015-08-10 17:09:08 UTC) #19
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 4 months ago (2015-08-10 17:45:54 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1278413002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1278413002/80001
5 years, 4 months ago (2015-08-10 18:20:31 UTC) #24
commit-bot: I haz the power
Committed patchset #5 (id:80001)
5 years, 4 months ago (2015-08-10 18:22:12 UTC) #25
commit-bot: I haz the power
5 years, 4 months ago (2015-08-10 18:22:28 UTC) #26
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/cc74437ba70633fcee817a45aca74b41fec37d38
Cr-Commit-Position: refs/heads/master@{#30096}

Powered by Google App Engine
This is Rietveld 408576698