• Benedikt Meurer's avatar
    [ic] Don't unroll the loop in AccessorAssembler::HandlePolymorphicCase(). · e004fe75
    Benedikt Meurer authored
    Previously AccessorAssembler::HandlePolymorphicCase() had 4 versions of
    the inner loop unrolled, but we always had to check against the length
    after 1 (POLYMORPHIC with name) or 2 (regular POLYMORPHIC) unrolled
    iterations anyways, so there's not a lot of benefit to unrolling besides
    the potentially better branch prediction in some cases. But that doesn't
    seem to be beneficial even in extreme cases (in fact on ARM cores we
    might get some benefit from having less code instead), and probably
    doesn't justify the additional C++ / generated code.
    
    I used the following extreme micro-benchmark to check the worst case
    performance impact:
    
    ```js
    function test(o, n) {
      var result;
      for (var i = 0; i < n; ++i) {
        result = o.x;
      }
      return result;
    }
    
    const N = 1e8;
    const objs = [{x: 0}, {x:1,a:1}, {x:2,b:2}, {x:3,c:3}];
    for (var j = 0; j < objs.length; ++j) test(objs[j], N);
    
    console.time('Time');
    for (var j = 0; j < objs.length; ++j) test(objs[j], N);
    console.timeEnd('Time');
    ```
    
    Running this with --noopt shows a ~1% performance regression with this
    patch on a beefy z840 gLinux workstation, which gives me some confidence
    that overall this patch is going to be neutral and maybe beneficial in
    case of less powerful ARM cores.
    
    Note to performance sheriffs: This could potentially tank some
    performance tests. In that case we may need to revisit the unrolling.
    
    Bug: v8:8562
    Change-Id: I731599a7778da1992d981d36022c407ef5c735eb
    Reviewed-on: https://chromium-review.googlesource.com/c/1448275Reviewed-by: 's avatarIgor Sheludko <ishell@chromium.org>
    Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#59252}
    e004fe75
accessor-assembler.h 15.6 KB