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

Issue 774473004: Use weak cells to embed maps in store handler. (Closed)

Created:
6 years ago by ulan
Modified:
6 years ago
Reviewers:
Toon Verwaest
CC:
v8-dev, adamk
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Project:
v8
Visibility:
Public.

Description

Use weak cells to embed maps in store handler. BUG=v8:3629 LOG=N

Patch Set 1 #

Patch Set 2 : Fix arm64 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+69 lines, -81 lines) Patch
M src/arm/macro-assembler-arm.h View 1 chunk +0 lines, -4 lines 0 comments Download
M src/arm/macro-assembler-arm.cc View 1 chunk +0 lines, -12 lines 0 comments Download
M src/arm64/macro-assembler-arm64.h View 1 chunk +0 lines, -4 lines 0 comments Download
M src/arm64/macro-assembler-arm64.cc View 1 1 chunk +0 lines, -11 lines 0 comments Download
M src/ia32/macro-assembler-ia32.h View 1 chunk +0 lines, -4 lines 0 comments Download
M src/ia32/macro-assembler-ia32.cc View 1 chunk +0 lines, -12 lines 0 comments Download
M src/ic/arm/handler-compiler-arm.cc View 1 chunk +16 lines, -3 lines 0 comments Download
M src/ic/arm64/handler-compiler-arm64.cc View 1 chunk +15 lines, -3 lines 0 comments Download
M src/ic/handler-compiler.h View 1 chunk +3 lines, -1 line 0 comments Download
M src/ic/handler-compiler.cc View 3 chunks +4 lines, -5 lines 0 comments Download
M src/ic/ia32/handler-compiler-ia32.cc View 1 chunk +16 lines, -3 lines 0 comments Download
M src/ic/x64/handler-compiler-x64.cc View 1 chunk +15 lines, -3 lines 0 comments Download
M src/x64/macro-assembler-x64.h View 1 chunk +0 lines, -4 lines 0 comments Download
M src/x64/macro-assembler-x64.cc View 1 chunk +0 lines, -12 lines 0 comments Download

Messages

Total messages: 12 (4 generated)
ulan
PTAL
6 years ago (2014-12-03 12:26:02 UTC) #2
Toon Verwaest
lgtm
6 years ago (2014-12-03 12:29:04 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/774473004/1
6 years ago (2014-12-03 12:49:02 UTC) #5
commit-bot: I haz the power
Try jobs failed on following builders: v8_linux_arm64_rel on tryserver.v8 (http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/1515)
6 years ago (2014-12-03 13:07:35 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/774473004/20001
6 years ago (2014-12-03 13:11:13 UTC) #9
commit-bot: I haz the power
Committed patchset #2 (id:20001)
6 years ago (2014-12-03 13:36:54 UTC) #10
adamk
On 2014/12/03 13:36:54, I haz the power (commit-bot) wrote: > Committed patchset #2 (id:20001) This ...
6 years ago (2014-12-04 21:27:01 UTC) #11
ulan
6 years ago (2014-12-05 13:00:34 UTC) #12
Message was sent while issue was closed.
> I can reproduce with the commandline:
> 
> out/arm64.debug/d8 --test --random-seed=1979322950 --stress-opt --always-opt
> --nohard-abort --nodead-code-elimination --nofold-constants
> --enable-slow-asserts --debug-code --verify-heap --expose-debug-as debug
> --expose-gc test/mjsunit/mjsunit.js test/mjsunit/es6/mirror-collections.js
> --gc-interval=500 --stress-compaction
--concurrent-recompilation-queue-length=64
> --concurrent-recompilation-delay=500 --concurrent-recompilation
> 
> and reverting this change clears up the failure.

This uncovered an existing bug. The fix is in review:
https://codereview.chromium.org/783673002/

Powered by Google App Engine
This is Rietveld 408576698