• Caitlin Potter's avatar
    [builtins] put SetPropertyInLiteral in a code-stub · ac3f98d5
    Caitlin Potter authored
    There are several core changes in this stub:
    
    1) add a version of KeyedStoreGenericGenerator::SetPropertyInLiteral()
    which supports indexed properties directly, witthout KeyedStore
    
    2) add a code stub for SetPropertyInLiteral which uses the version
    supporting indexed properties
    
    3) Use the code stub in CloneObjectIC, rather than using the smaller
    special-cased version which does not handle Names.
    
    Item 1) involves a refactoring which adds a nice way to reuse code in
    KeyedStoreGenericAssembler, which allows deleting a bunch of copy/pasted
    code. This makes it easy to reuse the index handling in
    KeyedStoreGeneric() without adding adding a bunch more duplicated
    handling. Because of this, I consider this to be somewhat of a cleanup,
    though if the copied code is preferred, I'm happy to revert to that.
    
    Item 2) is needed for Object.fromEntries(), as it's better to not
    require falling back to the slow path if a key happens to be an Smi ---
    but this is also optional.
    
    Item 3) benefits the codebase by allowing Object.fromEntries() to use
    this fast path without calling into the runtime, and without duplicating
    code which is also used by CloneObjectIC.
    
    I am skeptical that this should affect performance significantly. I've
    run ObjectLiteralSpread tests, and the mean of scores over 100 runs is
    somewhat surprising: CloneObjectIC --- the only user of this code,
    has an increased average score, while the polyfill cases score slightly
    worse --- However, the overall changes are small and likely flukes.
    The complete processed test output is below:
    
    ```
    // Mean of 100 runs of each benchmark
    
    Babel-ObjectLiteralSpread:
    -----+---------------------------+---------------------------+-------
         | With patch                | Without patch             | diff
    Mean | 11530.87                  | 12142.92                  | -5.04%
    -----+---------------------------+---------------------------+-------
    
    BabelAndOverwrite-ObjectLiteralSpread:
    -----+---------------------------+---------------------------+-------
         | With patch                | Without patch             | diff
    Mean | 10881.41                  | 11260.81                  | -3.37%
    -----+---------------------------+---------------------------+-------
    
    ObjectAssign-ObjectLiteralSpread:
    -----+---------------------------+---------------------------+-------
         | With patch                | Without patch             | diff
    Mean | 6188.92                   | 6358.55                   | -2.67%
    -----+---------------------------+---------------------------+-------
    
    ObjectAssignAndOverwrite-ObjectLiteralSpread:
    -----+---------------------------+---------------------------+-------
         | With patch                | Without patch             | diff
    Mean | 6112.80                   | 6275.54                   | -1.61%
    -----+---------------------------+---------------------------+-------
    
    ObjectSpread-ObjectLiteralSpread:
    -----+---------------------------+---------------------------+-------
         | With patch                | Without patch             | diff
    Mean | 51942.93                  | 50713.17                  | +3.46%
    -----+---------------------------+---------------------------+-------
    
    ObjectSpreadAndOverwrite-ObjectLiteralSpread:
    -----+---------------------------+---------------------------+-------
         | With patch                | Without patch             | diff
    Mean | 51375.23                  | 50833.29                  | +2.09%
    -----+---------------------------+---------------------------+-------
    ```
    
    BUG=v8:8238, v8:8021
    R=ishell@chromium.org, jkummerow@chromium.org
    
    Change-Id: I43e102fc461ffd389b5d6810a73f86e5012d7dee
    Reviewed-on: https://chromium-review.googlesource.com/c/1277751
    Commit-Queue: Caitlin Potter <caitp@igalia.com>
    Reviewed-by: 's avatarJakob Kummerow <jkummerow@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#56957}
    ac3f98d5
Name
Last commit
Last update
benchmarks Loading commit data...
build_overrides Loading commit data...
custom_deps Loading commit data...
docs Loading commit data...
gni Loading commit data...
include Loading commit data...
infra Loading commit data...
samples Loading commit data...
src Loading commit data...
test Loading commit data...
testing Loading commit data...
third_party Loading commit data...
tools Loading commit data...
.clang-format Loading commit data...
.clang-tidy Loading commit data...
.editorconfig Loading commit data...
.git-blame-ignore-revs Loading commit data...
.gitattributes Loading commit data...
.gitignore Loading commit data...
.gn Loading commit data...
.vpython Loading commit data...
.ycm_extra_conf.py Loading commit data...
AUTHORS Loading commit data...
BUILD.gn Loading commit data...
CODE_OF_CONDUCT.md Loading commit data...
ChangeLog Loading commit data...
DEPS Loading commit data...
LICENSE Loading commit data...
LICENSE.fdlibm Loading commit data...
LICENSE.strongtalk Loading commit data...
LICENSE.v8 Loading commit data...
LICENSE.valgrind Loading commit data...
OWNERS Loading commit data...
PRESUBMIT.py Loading commit data...
README.md Loading commit data...
WATCHLISTS Loading commit data...
codereview.settings Loading commit data...
snapshot_toolchain.gni Loading commit data...