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

Issue 7230047: Remove the fcontext field from all contexts. (Closed)

Created:
9 years, 5 months ago by Kevin Millikin (Chromium)
Modified:
9 years, 5 months ago
CC:
v8-dev
Visibility:
Public.

Description

Remove the fcontext field from all contexts. Before: every context cached the nearest enclosing function context. This assumed that for nested contexts (i.e., with and catch contexts) the enclosing function had a materialized link in the context chain. Now: when necessary, we loop up the context chain to find such a context. This enables catch contexts without forcing the enclosing function to allocate its own context. R=ager@chromium.org BUG= TEST= Committed: http://code.google.com/p/v8/source/detail?r=8452

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+97 lines, -139 lines) Patch
M src/arm/code-stubs-arm.cc View 1 chunk +0 lines, -1 line 0 comments Download
M src/arm/full-codegen-arm.cc View 2 chunks +7 lines, -16 lines 2 comments Download
M src/arm/macro-assembler-arm.cc View 1 chunk +0 lines, -11 lines 0 comments Download
M src/bootstrapper.cc View 1 chunk +0 lines, -1 line 0 comments Download
M src/contexts.h View 5 chunks +4 lines, -21 lines 0 comments Download
M src/contexts.cc View 2 chunks +23 lines, -11 lines 0 comments Download
M src/heap.cc View 3 chunks +0 lines, -3 lines 0 comments Download
M src/ia32/code-stubs-ia32.cc View 1 chunk +0 lines, -1 line 0 comments Download
M src/ia32/full-codegen-ia32.cc View 2 chunks +7 lines, -15 lines 0 comments Download
M src/ia32/macro-assembler-ia32.cc View 1 chunk +10 lines, -7 lines 0 comments Download
M src/runtime.cc View 8 chunks +27 lines, -28 lines 0 comments Download
M src/x64/code-stubs-x64.cc View 1 chunk +0 lines, -1 line 0 comments Download
M src/x64/full-codegen-x64.cc View 2 chunks +9 lines, -16 lines 0 comments Download
M src/x64/macro-assembler-x64.cc View 1 chunk +10 lines, -7 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Kevin Millikin (Chromium)
9 years, 5 months ago (2011-06-28 14:34:38 UTC) #1
Kevin Millikin (Chromium)
http://codereview.chromium.org/7230047/diff/1/src/arm/full-codegen-arm.cc File src/arm/full-codegen-arm.cc (right): http://codereview.chromium.org/7230047/diff/1/src/arm/full-codegen-arm.cc#newcode1872 src/arm/full-codegen-arm.cc:1872: case Slot::CONTEXT: Const initialization can be nested inside a ...
9 years, 5 months ago (2011-06-28 14:37:47 UTC) #2
fschneider
LGTM.
9 years, 5 months ago (2011-06-28 15:09:37 UTC) #3
Mads Ager (chromium)
9 years, 5 months ago (2011-06-29 05:56:27 UTC) #4
LGTM too

http://codereview.chromium.org/7230047/diff/1/src/arm/full-codegen-arm.cc
File src/arm/full-codegen-arm.cc (right):

http://codereview.chromium.org/7230047/diff/1/src/arm/full-codegen-arm.cc#new...
src/arm/full-codegen-arm.cc:1872: case Slot::CONTEXT:
On 2011/06/28 14:37:48, Kevin Millikin wrote:
> Const initialization can be nested inside a with or catch, but with our
current
> semantics they are hoisted.
> 
> We could implement the loop up the context chain in generated code (even
unroll
> the loop once the chain length is known statically), but I've opted to just
call
> the runtime.

Let's file a bug report (internal cleanup?) about investigating the impact of
native code implementation of this to not forget. Context stores could be quite
frequent and this would slow it down quite a bit.

Powered by Google App Engine
This is Rietveld 408576698