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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1305223006: Remove allocation in old space .... (Closed)

Created:
5 years, 3 months ago by srdjan
Modified:
5 years, 3 months ago
Reviewers:
regis
CC:
reviews_dartlang.org, vm-dev_dartlang.org
Base URL:
https://github.com/dart-lang/sdk.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Remove allocation in old space .... Eliminate need for GrowableObjectArray, allocate constants in old space. BUG= R=regis@google.com Committed: https://github.com/dart-lang/sdk/commit/7fb555045a436e11e0c324a4ea9cf1b90e78d237

Patch Set 1 #

Total comments: 4

Patch Set 2 : d #

Patch Set 3 : Formatting #

Unified diffs Side-by-side diffs Delta from patch set Stats (+112 lines, -96 lines) Patch
M runtime/vm/class_finalizer.h View 4 chunks +7 lines, -5 lines 0 comments Download
M runtime/vm/class_finalizer.cc View 1 8 chunks +10 lines, -15 lines 0 comments Download
M runtime/vm/compiler.cc View 1 2 7 chunks +24 lines, -30 lines 0 comments Download
M runtime/vm/intermediate_language.cc View 2 chunks +4 lines, -2 lines 0 comments Download
M runtime/vm/object.h View 7 chunks +15 lines, -7 lines 0 comments Download
M runtime/vm/object.cc View 1 18 chunks +47 lines, -33 lines 0 comments Download
M runtime/vm/parser.cc View 1 1 chunk +5 lines, -4 lines 0 comments Download

Messages

Total messages: 5 (1 generated)
srdjan
5 years, 3 months ago (2015-09-01 23:10:20 UTC) #2
regis
LGTM https://codereview.chromium.org/1305223006/diff/1/runtime/vm/compiler.cc File runtime/vm/compiler.cc (right): https://codereview.chromium.org/1305223006/diff/1/runtime/vm/compiler.cc#newcode215 runtime/vm/compiler.cc:215: extra blank line https://codereview.chromium.org/1305223006/diff/1/runtime/vm/compiler.cc#newcode296 runtime/vm/compiler.cc:296: StackZone zone(thread); By ...
5 years, 3 months ago (2015-09-01 23:25:08 UTC) #3
srdjan
Committed patchset #3 (id:40001) manually as 7fb555045a436e11e0c324a4ea9cf1b90e78d237 (presubmit successful).
5 years, 3 months ago (2015-09-02 16:00:00 UTC) #4
srdjan
5 years, 3 months ago (2015-09-02 16:01:12 UTC) #5
Message was sent while issue was closed. 
https://codereview.chromium.org/1305223006/diff/1/runtime/vm/compiler.cc
File runtime/vm/compiler.cc (right):

https://codereview.chromium.org/1305223006/diff/1/runtime/vm/compiler.cc#newc...
runtime/vm/compiler.cc:215: 
On 2015/09/01 23:25:08, regis wrote:
> extra blank line

Done.

https://codereview.chromium.org/1305223006/diff/1/runtime/vm/compiler.cc#newc...
runtime/vm/compiler.cc:296: StackZone zone(thread);
On 2015/09/01 23:25:08, regis wrote:
> By moving this declaration here, the stack zone will not be freed in case of a
> longjump. Is it what we want? Maybe it was not correct before, since we were
> accessing this zone in case of an error?

I think that is safer now. I had to move it so that the active zone is the same
as the one used inside GrowableHandlePtrArray-s.

Powered by Google App Engine
This is Rietveld 408576698