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

Issue 2674593003: [TypeFeedbackVector] Root feedback vectors at function literal site. (Closed)

Created:
3 years, 10 months ago by mvstanton
Modified:
3 years, 10 months ago
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
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[TypeFeedbackVector] Root feedback vectors at function literal site. TypeFeedbackVectors are strongly rooted by a closure. However, in modern JavaScript closures are created and abandoned more freely. An important closure may not be present in the root-set at time of garbage collection, even though we've cached optimized code and use it regularly. For example, consider leaf functions in an event dispatching system. They may well be "hot," but tragically non-present when we collect the heap. Until now, we've relied on a weak root to cache the feedback vector in this case. Since there is no way to signal intent or relative importance, this weak root is as susceptible to clearing as any other weak root at garbage collection time. Meanwhile, the feedback vector has become more important. All of our ICs store their data there. Literal and regex boilerplates are stored there. If we lose the vector, then we not only lose optimized code built from it, we also lose the very feedback which allowed us to create that optimized code. Therefore it's vital to express that dependency through the root set. This CL does this by creating a strong link to a feedback vector at the instantiation site of the function closure. This instantiation site is in the code and feedback vector of the outer closure. BUG=v8:5456 Review-Url: https://codereview.chromium.org/2674593003 Cr-Commit-Position: refs/heads/master@{#42953} Committed: https://chromium.googlesource.com/v8/v8/+/aea3ce3df3dc8838940e5db74c9d459a3ba999e3

Patch Set 1 #

Patch Set 2 : Ports. #

Patch Set 3 : REBASE. #

Total comments: 13

Patch Set 4 : Fixes, comments. #

Patch Set 5 : A few fixes. #

Patch Set 6 : Liveedit bug fix. #

Patch Set 7 : REBASE+ports update. #

Patch Set 8 : REBASE+liveedit fix. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+805 lines, -836 lines) Patch
M src/arm/macro-assembler-arm.cc View 7 1 chunk +1 line, -0 lines 0 comments Download
M src/arm64/macro-assembler-arm64.cc View 7 1 chunk +1 line, -0 lines 0 comments Download
M src/builtins/arm/builtins-arm.cc View 1 2 3 5 chunks +13 lines, -22 lines 0 comments Download
M src/builtins/arm64/builtins-arm64.cc View 1 2 3 5 chunks +10 lines, -17 lines 0 comments Download
M src/builtins/builtins-constructor.cc View 4 chunks +13 lines, -13 lines 0 comments Download
M src/builtins/ia32/builtins-ia32.cc View 5 chunks +9 lines, -19 lines 0 comments Download
M src/builtins/mips/builtins-mips.cc View 1 2 3 5 chunks +13 lines, -22 lines 0 comments Download
M src/builtins/mips64/builtins-mips64.cc View 1 2 3 5 chunks +13 lines, -22 lines 0 comments Download
M src/builtins/ppc/builtins-ppc.cc View 1 2 3 4 5 6 5 chunks +10 lines, -17 lines 0 comments Download
M src/builtins/s390/builtins-s390.cc View 1 2 3 4 5 6 5 chunks +10 lines, -16 lines 0 comments Download
M src/builtins/x64/builtins-x64.cc View 1 2 3 5 chunks +10 lines, -17 lines 0 comments Download
M src/builtins/x87/builtins-x87.cc View 1 2 3 4 5 6 5 chunks +9 lines, -18 lines 0 comments Download
M src/code-stub-assembler.cc View 1 2 3 4 5 6 7 2 chunks +3 lines, -2 lines 0 comments Download
M src/compilation-cache.h View 5 chunks +32 lines, -29 lines 0 comments Download
M src/compilation-cache.cc View 1 2 3 4 6 chunks +71 lines, -82 lines 0 comments Download
M src/compiler.cc View 1 2 3 4 10 chunks +77 lines, -41 lines 0 comments Download
M src/compiler/js-create-lowering.cc View 2 chunks +5 lines, -2 lines 0 comments Download
M src/compiler/js-graph.h View 2 chunks +0 lines, -2 lines 0 comments Download
M src/compiler/js-graph.cc View 1 chunk +0 lines, -5 lines 0 comments Download
M src/compiler/js-native-context-specialization.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M src/contexts.h View 1 2 3 4 5 6 1 chunk +2 lines, -3 lines 0 comments Download
M src/contexts.cc View 6 chunks +8 lines, -27 lines 0 comments Download
M src/debug/liveedit.cc View 1 2 3 4 5 6 7 5 chunks +23 lines, -44 lines 0 comments Download
M src/deoptimizer.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M src/factory.h View 1 chunk +2 lines, -2 lines 0 comments Download
M src/factory.cc View 1 2 3 5 chunks +4 lines, -7 lines 0 comments Download
M src/full-codegen/arm/full-codegen-arm.cc View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M src/full-codegen/arm64/full-codegen-arm64.cc View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M src/full-codegen/ia32/full-codegen-ia32.cc View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M src/full-codegen/mips/full-codegen-mips.cc View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M src/full-codegen/mips64/full-codegen-mips64.cc View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M src/full-codegen/ppc/full-codegen-ppc.cc View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M src/full-codegen/s390/full-codegen-s390.cc View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M src/full-codegen/x64/full-codegen-x64.cc View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M src/full-codegen/x87/full-codegen-x87.cc View 1 2 3 4 5 6 1 chunk +2 lines, -2 lines 0 comments Download
M src/handles-inl.h View 1 chunk +0 lines, -1 line 0 comments Download
M src/heap/object-stats.cc View 1 chunk +5 lines, -21 lines 0 comments Download
M src/ia32/macro-assembler-ia32.cc View 7 1 chunk +1 line, -0 lines 0 comments Download
M src/interpreter/interpreter-assembler.cc View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M src/mips/macro-assembler-mips.cc View 7 1 chunk +1 line, -0 lines 0 comments Download
M src/mips64/macro-assembler-mips64.cc View 7 1 chunk +1 line, -0 lines 0 comments Download
M src/objects.h View 1 2 3 4 5 6 7 chunks +37 lines, -21 lines 0 comments Download
M src/objects.cc View 1 2 3 4 13 chunks +230 lines, -80 lines 0 comments Download
M src/objects-inl.h View 1 2 3 4 5 6 3 chunks +13 lines, -2 lines 0 comments Download
M src/objects-printer.cc View 1 2 3 4 5 6 7 2 chunks +9 lines, -1 line 0 comments Download
M src/ppc/macro-assembler-ppc.cc View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M src/profiler/heap-snapshot-generator.cc View 1 2 3 4 5 6 7 1 chunk +4 lines, -3 lines 0 comments Download
M src/runtime/runtime-interpreter.cc View 1 chunk +5 lines, -1 line 0 comments Download
M src/runtime/runtime-scopes.cc View 1 2 3 4 5 6 7 4 chunks +19 lines, -5 lines 0 comments Download
M src/s390/macro-assembler-s390.cc View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M src/type-feedback-vector.cc View 1 2 3 4 5 6 7 3 chunks +12 lines, -16 lines 0 comments Download
M src/x64/macro-assembler-x64.cc View 7 1 chunk +1 line, -0 lines 0 comments Download
M src/x87/macro-assembler-x87.cc View 1 2 3 4 5 6 1 chunk +2 lines, -2 lines 0 comments Download
M test/cctest/heap/test-heap.cc View 1 2 3 4 5 6 4 chunks +12 lines, -216 lines 0 comments Download
M test/cctest/interpreter/interpreter-tester.h View 1 chunk +0 lines, -3 lines 0 comments Download
M test/cctest/test-code-stub-assembler.cc View 1 chunk +1 line, -2 lines 0 comments Download
M test/cctest/test-compiler.cc View 1 chunk +0 lines, -1 line 0 comments Download
M test/cctest/test-feedback-vector.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -2 lines 0 comments Download
M test/cctest/test-heap-profiler.cc View 1 chunk +12 lines, -8 lines 0 comments Download
M test/cctest/test-serialize.cc View 1 2 3 4 3 chunks +10 lines, -11 lines 0 comments Download
A test/mjsunit/strong-rooted-literals.js View 1 chunk +55 lines, -0 lines 0 comments Download
M test/unittests/interpreter/interpreter-assembler-unittest.cc View 1 chunk +7 lines, -5 lines 0 comments Download
M tools/gdbinit View 1 2 3 4 5 1 chunk +10 lines, -0 lines 0 comments Download

Messages

Total messages: 40 (31 generated)
mvstanton
Hi guys, Benedikt - can you review the whole thing. Much is simpler than last ...
3 years, 10 months ago (2017-02-02 12:05:27 UTC) #11
Benedikt Meurer
Leaving compiler and compilation cache review to Michi. Otherwise LGTM from my side. https://codereview.chromium.org/2674593003/diff/40001/src/handles-inl.h File ...
3 years, 10 months ago (2017-02-02 12:47:03 UTC) #12
Michael Starzinger
LGTM on compiler changes. If you want a more thorough look at the compilation cache ...
3 years, 10 months ago (2017-02-02 13:42:08 UTC) #13
Yang
liveedit lgtm.
3 years, 10 months ago (2017-02-02 14:11:33 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2674593003/120001
3 years, 10 months ago (2017-02-03 12:15:44 UTC) #27
mvstanton
Thanks everyone! Landing... --Michael https://codereview.chromium.org/2674593003/diff/40001/src/debug/liveedit.cc File src/debug/liveedit.cc (right): https://codereview.chromium.org/2674593003/diff/40001/src/debug/liveedit.cc#newcode800 src/debug/liveedit.cc:800: static void IterateAllJSFunctions(Heap* heap, Visitor* ...
3 years, 10 months ago (2017-02-03 12:16:37 UTC) #28
commit-bot: I haz the power
Try jobs failed on following builders: v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/20403) v8_linux64_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, ...
3 years, 10 months ago (2017-02-03 12:55:07 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2674593003/140001
3 years, 10 months ago (2017-02-06 10:15:17 UTC) #37
commit-bot: I haz the power
3 years, 10 months ago (2017-02-06 10:18:13 UTC) #40
Message was sent while issue was closed.
Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/v8/v8/+/aea3ce3df3dc8838940e5db74c9d459a3ba...

Powered by Google App Engine
This is Rietveld 408576698