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

Issue 348313002: Introduce a PrototypeIterator template and use it all over the place (Closed)

Created:
6 years, 6 months ago by jochen (gone - plz use gerrit)
Modified:
6 years, 5 months ago
Reviewers:
dcarney, Toon Verwaest
CC:
v8-dev
Visibility:
Public.

Description

Introduce a PrototypeIterator template and use it all over the place The idea is to have one central place where all prototype access happens. The different template parameters specialize on - How to objects are stored (raw pointer vs handle) - Whether we walk the prototype chain based on the map, or via a type switch - Where the prototype chain ends (null value, non hidden prototype, given object) A macro is provided for the common case that only the first prototype is accessed. BUG=??? R=dcarney@chromium.org,verwaest@chromium.org LOG=n

Patch Set 1 #

Patch Set 2 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+499 lines, -198 lines) Patch
M BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M src/accessors.cc View 3 chunks +6 lines, -4 lines 0 comments Download
M src/api.cc View 3 chunks +10 lines, -6 lines 0 comments Download
M src/builtins.cc View 5 chunks +8 lines, -9 lines 0 comments Download
M src/deoptimizer.cc View 2 chunks +2 lines, -1 line 0 comments Download
M src/heap-snapshot-generator.cc View 2 chunks +2 lines, -1 line 0 comments Download
M src/hydrogen.cc View 2 chunks +5 lines, -4 lines 0 comments Download
M src/ic.cc View 5 chunks +5 lines, -4 lines 0 comments Download
M src/ic-inl.h View 2 chunks +2 lines, -1 line 0 comments Download
M src/isolate.cc View 1 2 chunks +6 lines, -6 lines 0 comments Download
M src/json-stringifier.h View 2 chunks +3 lines, -2 lines 0 comments Download
M src/objects.h View 1 2 chunks +8 lines, -2 lines 0 comments Download
M src/objects.cc View 35 chunks +81 lines, -94 lines 0 comments Download
M src/objects-inl.h View 2 chunks +2 lines, -1 line 0 comments Download
M src/objects-printer.cc View 2 chunks +2 lines, -1 line 0 comments Download
A src/prototype-iterator.h View 1 chunk +291 lines, -0 lines 0 comments Download
M src/runtime.cc View 1 17 chunks +49 lines, -51 lines 0 comments Download
M src/string-stream.cc View 2 chunks +5 lines, -5 lines 0 comments Download
M src/stub-cache.cc View 4 chunks +10 lines, -6 lines 0 comments Download
M tools/gyp/v8.gyp View 1 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
jochen (gone - plz use gerrit)
ptal I haven't converted cctests and map->prototype() yet
6 years, 6 months ago (2014-06-24 09:53:19 UTC) #1
Toon Verwaest
Can't you derive STORE_AS_HANDLE / STORE_AS_POINTER from the input type? I find the names TYPE_BASED_WALK ...
6 years, 6 months ago (2014-06-25 12:44:40 UTC) #2
jochen (gone - plz use gerrit)
6 years, 6 months ago (2014-06-25 13:41:48 UTC) #3
On 2014/06/25 at 12:44:40, verwaest wrote:
> Can't you derive STORE_AS_HANDLE / STORE_AS_POINTER from the input type?

I'd need two fields in the class instead of one.

All methods (IsAtEnd/Advance) have different implementations, so I'd always need
to check which field is used.

I fear the compilers won't optimize this away in all cases.

> 
> I find the names TYPE_BASED_WALK / MAP_BASED_WALK quite confusing. They are
just artifacts from our current implementation right?
> What I think you want is to just have 2 versions: one which takes JSObjects as
input, and one which takes Objects as input. From Object we first derive the
"root" as I did in lookup.[h|cc], and then walk the prototype JSObject-based
chain starting from that root. So we only need to use GetPrototype(Isolate*)
once for translating Object* to JSObject.
> 
> The result of GetCurrent() probably should be a JSReceiver since it could be a
proxy. When current is JSProxy, we are at end as well.
> 
> The only actual manual configuration we need, I think, is where to stop.

Powered by Google App Engine
This is Rietveld 408576698