Chromium Code Reviews

Issue 8491016: Refactoring only: Make the handling of PropertyType more explicit. (Closed)

Created:
9 years, 1 month ago by Sven Panne
Modified:
9 years, 1 month ago
Reviewers:
Kevin Millikin (Chromium)
CC:
v8-dev
Visibility:
Public.

Description

Refactoring only: Make the handling of PropertyType more explicit. Do not rely on 'default' clauses or 'if's when analysing a PropertyType, because this makes it hard to find the relevant places when a new type is added. Note that the detection of "phantom property types" is left untouched, because this might have a performance impact, especially for the GC (to be investigated). This is a preliminary step for introducing a new kind of map transition. Committed: http://code.google.com/p/v8/source/detail?r=9900

Patch Set 1 #

Total comments: 6

Patch Set 2 : Unified our 2 UpdateCaches implementations, making some assumptions more explicit #

Unified diffs Side-by-side diffs Stats (+74 lines, -43 lines)
M src/ic.cc View 7 chunks +32 lines, -21 lines 0 comments
M src/objects.h View 1 chunk +1 line, -2 lines 0 comments
M src/objects.cc View 6 chunks +12 lines, -12 lines 0 comments
M src/objects-inl.h View 1 chunk +1 line, -3 lines 0 comments
M src/objects-printer.cc View 1 chunk +3 lines, -1 line 0 comments
M src/property.h View 1 chunk +2 lines, -2 lines 0 comments
M src/runtime.cc View 1 chunk +3 lines, -2 lines 0 comments
M src/v8globals.h View 2 chunks +20 lines, -0 lines 0 comments

Messages

Total messages: 4 (0 generated)
Sven Panne
9 years, 1 month ago (2011-11-07 13:32:10 UTC) #1
Kevin Millikin (Chromium)
I don't immediately see why some of the types are unreachable in KeyedStoreIC::UpdateCaches. http://codereview.chromium.org/8491016/diff/1/src/ic.cc File ...
9 years, 1 month ago (2011-11-07 14:13:39 UTC) #2
Sven Panne
I've addressed the "impossible cases" parts of the review, the misplacement of PropertyType and its ...
9 years, 1 month ago (2011-11-08 07:36:09 UTC) #3
Kevin Millikin (Chromium)
9 years, 1 month ago (2011-11-08 08:26:23 UTC) #4
LGTM.

Powered by Google App Engine