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

Issue 1991173002: [keys] Don't omit write barrier after std::sort (Closed)

Created:
4 years, 7 months ago by Camillo Bruni
Modified:
4 years, 7 months ago
Reviewers:
Michael Lippautz
CC:
v8-reviews_googlegroups.com
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[keys] Don't omit write barrier after std::sort BUG= Committed: https://crrev.com/681ac8979e820d147380b5fdb2aae773e22f1302 Cr-Commit-Position: refs/heads/master@{#36412}

Patch Set 1 #

Patch Set 2 : fixing isolate ref #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -0 lines) Patch
M src/objects.cc View 1 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 9 (3 generated)
Camillo Bruni
PTAL, AFIACT this would be another candidate for nice errors ;)
4 years, 7 months ago (2016-05-20 09:28:46 UTC) #2
Michael Lippautz
lgtm
4 years, 7 months ago (2016-05-20 11:28:39 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1991173002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1991173002/20001
4 years, 7 months ago (2016-05-20 13:21:55 UTC) #5
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 7 months ago (2016-05-20 13:57:26 UTC) #6
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/681ac8979e820d147380b5fdb2aae773e22f1302 Cr-Commit-Position: refs/heads/master@{#36412}
4 years, 7 months ago (2016-05-20 13:59:57 UTC) #8
Jakob Kummerow
4 years, 7 months ago (2016-05-20 14:12:15 UTC) #9
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/1996963002/ by jkummerow@chromium.org.

The reason for reverting is: This array only contains Smis, see its only store
site five lines above:

array->set(array_size++, Smi::FromInt(i));

If you want to improve something here, use a C++ array instead of a FixedArray.
There's no reason to have this short-lived list on the V8 heap..

Powered by Google App Engine
This is Rietveld 408576698