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

Issue 212673011: Fix property enum cache creation to include only own properties (Closed)

Created:
6 years, 9 months ago by Jarin
Modified:
6 years, 9 months ago
Reviewers:
Toon Verwaest
CC:
v8-dev
Visibility:
Public.

Description

With this fix, we only create the enum cache for own property descriptors (originally we cached all descriptors in the map). The problem was that the size of all descriptors could be trimmed during GC triggered by allocating the storage for the cache, so we could have ended up with a wrong storage size. This is really Toon's fix, I have only created a small repro case. BUG= R=verwaest@chromium.org Committed: https://code.google.com/p/v8/source/detail?r=20308

Patch Set 1 #

Total comments: 2

Patch Set 2 : Tweaks #

Unified diffs Side-by-side diffs Delta from patch set Stats (+42 lines, -23 lines) Patch
M src/handles.cc View 1 4 chunks +23 lines, -23 lines 0 comments Download
A test/mjsunit/regress/regress-enum-prop-keys-cache-size.js View 1 chunk +19 lines, -0 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Jarin
6 years, 9 months ago (2014-03-27 09:01:14 UTC) #1
Toon Verwaest
lgtm with 2 nits https://codereview.chromium.org/212673011/diff/1/src/handles.cc File src/handles.cc (right): https://codereview.chromium.org/212673011/diff/1/src/handles.cc#newcode666 src/handles.cc:666: int num_enum = map->NumberOfDescribedProperties(OWN_DESCRIPTORS, DONT_SHOW); ...
6 years, 9 months ago (2014-03-27 11:53:38 UTC) #2
Jarin
6 years, 9 months ago (2014-03-27 15:33:14 UTC) #3
Message was sent while issue was closed.
Committed patchset #2 manually as r20308 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698