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

Issue 1668103002: Type Feedback Vector lives in the closure (Closed)

Created:
4 years, 10 months ago by mvstanton
Modified:
4 years, 10 months ago
Reviewers:
Benedikt Meurer
CC:
v8-reviews_googlegroups.com, v8-x87-ports_googlegroups.com, v8-ppc-ports_googlegroups.com, Yang, v8-mips-ports_googlegroups.com, rmcilroy, Hannes Payer (out of office), ulan, oth
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

Type Feedback Vector lives in the closure (RELAND: the problem before was a missing write barrier for adding the code entry to the new closure. It's been addressed with a new macro instruction and test. The only change to this CL is the addition of two calls to __ RecordWriteCodeEntryField() in the platform CompileLazy builtin.) We get less "pollution" of type feedback if we have one vector per native context, rather than one for the whole system. This CL moves the vector appropriately. We rely more heavily on the Optimized Code Map in the SharedFunctionInfo. The vector actually lives in the first slot of the literals array (indeed there is great commonality between those arrays, they can be thought of as the same thing). So we make greater effort to ensure there is a valid literals array after compilation. This meant, for performance reasons, that we needed to extend FastNewClosureStub to support creating closures with literals. And ultimately, it drove us to move the optimized code map lookup out of FastNewClosureStub and into the compile lazy builtin. The heap change is trivial so I TBR Hannes for it... Also, Yang has had a look at the debugger changes already and approved 'em. So he is TBR style too. And Benedikt reviewed it as well. TBR=hpayer@chromium.org, yangguo@chromium.org, bmeurer@chromium.org BUG= Committed: https://crrev.com/bb31db3ad6de16f86a61f6c7bbfd3274e3d957b5 Cr-Commit-Position: refs/heads/master@{#33741}

Patch Set 1 #

Patch Set 2 : REBASE. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1274 lines, -555 lines) Patch
M src/arm/builtins-arm.cc View 1 1 chunk +147 lines, -0 lines 0 comments Download
M src/arm/macro-assembler-arm.cc View 1 chunk +2 lines, -3 lines 0 comments Download
M src/arm64/builtins-arm64.cc View 1 1 chunk +131 lines, -0 lines 0 comments Download
M src/arm64/macro-assembler-arm64.cc View 1 chunk +2 lines, -3 lines 0 comments Download
M src/code-stubs.cc View 1 chunk +1 line, -5 lines 0 comments Download
M src/code-stubs-hydrogen.cc View 3 chunks +10 lines, -203 lines 0 comments Download
M src/compiler.h View 2 chunks +4 lines, -4 lines 0 comments Download
M src/compiler.cc View 15 chunks +36 lines, -26 lines 0 comments Download
M src/compiler/access-builder.h View 2 chunks +5 lines, -2 lines 0 comments Download
M src/compiler/access-builder.cc View 1 chunk +9 lines, -2 lines 0 comments Download
M src/compiler/ast-graph-builder.cc View 2 chunks +4 lines, -4 lines 0 comments Download
M src/compiler/bytecode-graph-builder.cc View 1 2 chunks +5 lines, -4 lines 0 comments Download
M src/compiler/interpreter-assembler.cc View 1 1 chunk +2 lines, -3 lines 0 comments Download
M src/compiler/js-inlining.cc View 1 chunk +3 lines, -0 lines 0 comments Download
M src/crankshaft/hydrogen.h View 1 chunk +4 lines, -1 line 0 comments Download
M src/crankshaft/hydrogen.cc View 2 chunks +5 lines, -2 lines 0 comments Download
M src/crankshaft/typing.cc View 2 chunks +1 line, -2 lines 0 comments Download
M src/debug/debug.cc View 2 chunks +3 lines, -4 lines 0 comments Download
M src/debug/liveedit.h View 1 chunk +1 line, -1 line 0 comments Download
M src/debug/liveedit.cc View 7 chunks +23 lines, -15 lines 0 comments Download
M src/deoptimizer.h View 1 chunk +0 lines, -1 line 0 comments Download
M src/factory.h View 1 chunk +1 line, -1 line 0 comments Download
M src/factory.cc View 4 chunks +8 lines, -10 lines 0 comments Download
M src/full-codegen/arm/full-codegen-arm.cc View 1 chunk +2 lines, -5 lines 0 comments Download
M src/full-codegen/arm64/full-codegen-arm64.cc View 1 chunk +2 lines, -5 lines 0 comments Download
M src/full-codegen/full-codegen.cc View 1 chunk +1 line, -1 line 0 comments Download
M src/full-codegen/ia32/full-codegen-ia32.cc View 1 chunk +2 lines, -5 lines 0 comments Download
M src/full-codegen/mips/full-codegen-mips.cc View 1 chunk +2 lines, -5 lines 0 comments Download
M src/full-codegen/mips64/full-codegen-mips64.cc View 1 chunk +2 lines, -5 lines 0 comments Download
M src/full-codegen/x64/full-codegen-x64.cc View 1 chunk +2 lines, -5 lines 0 comments Download
M src/heap/heap.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M src/heap/heap.cc View 1 1 chunk +8 lines, -0 lines 0 comments Download
M src/heap/objects-visiting-inl.h View 2 chunks +4 lines, -3 lines 0 comments Download
M src/ia32/builtins-ia32.cc View 1 1 chunk +140 lines, -0 lines 0 comments Download
M src/ia32/macro-assembler-ia32.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M src/interpreter/bytecode-generator.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M src/interpreter/interpreter.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M src/mips/builtins-mips.cc View 1 1 chunk +142 lines, -0 lines 0 comments Download
M src/mips/macro-assembler-mips.cc View 1 chunk +2 lines, -3 lines 0 comments Download
M src/mips64/builtins-mips64.cc View 1 1 chunk +142 lines, -0 lines 0 comments Download
M src/mips64/macro-assembler-mips64.cc View 1 chunk +2 lines, -3 lines 0 comments Download
M src/objects.h View 1 11 chunks +32 lines, -15 lines 0 comments Download
M src/objects.cc View 1 5 chunks +41 lines, -8 lines 0 comments Download
M src/objects-debug.cc View 1 chunk +1 line, -1 line 0 comments Download
M src/objects-inl.h View 1 5 chunks +37 lines, -5 lines 0 comments Download
M src/objects-printer.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M src/profiler/heap-snapshot-generator.cc View 1 1 chunk +3 lines, -3 lines 0 comments Download
M src/runtime-profiler.cc View 5 chunks +9 lines, -11 lines 0 comments Download
M src/runtime/runtime.h View 3 chunks +1 line, -2 lines 0 comments Download
M src/runtime/runtime-compiler.cc View 1 1 chunk +16 lines, -0 lines 0 comments Download
M src/runtime/runtime-function.cc View 2 chunks +6 lines, -8 lines 0 comments Download
M src/runtime/runtime-scopes.cc View 1 1 chunk +7 lines, -0 lines 0 comments Download
M src/runtime/runtime-test.cc View 1 chunk +1 line, -1 line 0 comments Download
M src/snapshot/serialize.cc View 1 1 chunk +9 lines, -2 lines 0 comments Download
M src/type-feedback-vector.h View 2 chunks +4 lines, -2 lines 0 comments Download
M src/type-feedback-vector.cc View 3 chunks +37 lines, -3 lines 0 comments Download
M src/type-feedback-vector-inl.h View 1 chunk +5 lines, -5 lines 0 comments Download
M src/x64/builtins-x64.cc View 1 1 chunk +128 lines, -0 lines 0 comments Download
M src/x64/macro-assembler-x64.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M test/cctest/cctest.status View 1 2 chunks +7 lines, -4 lines 0 comments Download
M test/cctest/compiler/function-tester.h View 2 chunks +2 lines, -0 lines 0 comments Download
M test/cctest/compiler/test-run-jscalls.cc View 2 chunks +2 lines, -0 lines 0 comments Download
M test/cctest/heap/test-heap.cc View 1 17 chunks +22 lines, -101 lines 0 comments Download
M test/cctest/interpreter/test-interpreter.cc View 1 chunk +1 line, -1 line 0 comments Download
M test/cctest/test-compiler.cc View 8 chunks +13 lines, -19 lines 0 comments Download
M test/cctest/test-feedback-vector.cc View 10 chunks +10 lines, -10 lines 0 comments Download
M test/mjsunit/mjsunit.status View 1 1 chunk +3 lines, -0 lines 0 comments Download
M test/unittests/compiler/interpreter-assembler-unittest.cc View 1 chunk +8 lines, -10 lines 0 comments Download

Messages

Total messages: 12 (7 generated)
mvstanton
Hi Benedikt, I've addressed the write barrier issue with another CL. This version adds two ...
4 years, 10 months ago (2016-02-04 10:27:49 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1668103002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1668103002/40001
4 years, 10 months ago (2016-02-04 15:39:02 UTC) #7
commit-bot: I haz the power
Committed patchset #2 (id:40001)
4 years, 10 months ago (2016-02-04 15:41:02 UTC) #9
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/bb31db3ad6de16f86a61f6c7bbfd3274e3d957b5 Cr-Commit-Position: refs/heads/master@{#33741}
4 years, 10 months ago (2016-02-04 15:41:29 UTC) #11
mvstanton
4 years, 10 months ago (2016-02-05 10:47:55 UTC) #12
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:40001) has been created in
https://codereview.chromium.org/1670813005/ by mvstanton@chromium.org.

The reason for reverting is: Must revert for now due to chromium api natives
issues..

Powered by Google App Engine
This is Rietveld 408576698