• 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
builtins-definitions.h 118 KB