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

Issue 10069050: Add isolate accessor to AccessorInfo and Arguments. (Closed)

Created:
8 years, 8 months ago by Michael Starzinger
Modified:
8 years, 8 months ago
Reviewers:
danno
CC:
v8-dev
Visibility:
Public.

Description

Add isolate accessor to AccessorInfo and Arguments. This passes the isolate through to API callback functions so that it is available through AccessorInfo and Arguments. This allows bindings to avoid unnecessary TLS lookups to retrieve the current isolate. R=danno@chromium.org TEST=cctest/test-api,cctest/test-accessors Committed: https://code.google.com/p/v8/source/detail?r=11306

Patch Set 1 #

Patch Set 2 : Fixed comments for ARM. #

Total comments: 2

Patch Set 3 : Addressed comment by Daniel Clifford. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+120 lines, -129 lines) Patch
M include/v8.h View 6 chunks +18 lines, -4 lines 0 comments Download
M src/apiutils.h View 1 2 3 chunks +7 lines, -2 lines 0 comments Download
M src/arguments.h View 3 chunks +7 lines, -5 lines 0 comments Download
M src/arm/stub-cache-arm.cc View 1 12 chunks +29 lines, -23 lines 0 comments Download
M src/builtins.h View 1 chunk +0 lines, -1 line 0 comments Download
M src/builtins.cc View 3 chunks +2 lines, -64 lines 0 comments Download
M src/ia32/stub-cache-ia32.cc View 11 chunks +18 lines, -12 lines 0 comments Download
M src/stub-cache.cc View 4 chunks +6 lines, -4 lines 0 comments Download
M src/x64/stub-cache-x64.cc View 12 chunks +19 lines, -13 lines 0 comments Download
M test/cctest/test-accessors.cc View 4 chunks +8 lines, -1 line 0 comments Download
M test/cctest/test-api.cc View 3 chunks +6 lines, -0 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Michael Starzinger
8 years, 8 months ago (2012-04-13 08:37:09 UTC) #1
danno
LGTM with comment https://chromiumcodereview.appspot.com/10069050/diff/2001/src/apiutils.h File src/apiutils.h (right): https://chromiumcodereview.appspot.com/10069050/diff/2001/src/apiutils.h#newcode61 src/apiutils.h:61: ASSERT(implicit_args[v8::Arguments::kIsolateIndex]->IsSmi()); Comment might be good here ...
8 years, 8 months ago (2012-04-13 09:00:34 UTC) #2
Michael Starzinger
8 years, 8 months ago (2012-04-13 09:38:38 UTC) #3
Added new patch set. Rebased. Landed.

https://chromiumcodereview.appspot.com/10069050/diff/2001/src/apiutils.h
File src/apiutils.h (right):

https://chromiumcodereview.appspot.com/10069050/diff/2001/src/apiutils.h#newc...
src/apiutils.h:61: ASSERT(implicit_args[v8::Arguments::kIsolateIndex]->IsSmi());
On 2012/04/13 09:00:34, danno wrote:
> Comment might be good here why it's a SMI

Done.

Powered by Google App Engine
This is Rietveld 408576698