t2ym / thin-hook

Thin Hook Preprocessor
Other
4 stars 1 forks source link

[demo][hook-callback][Chrome Canary 63.0.3239.0] Map.get() is very slow #155

Closed t2ym closed 7 years ago

t2ym commented 7 years ago

[demo][hook-callback][Chrome Canary 63.0.3239.0] Map.get() is very slow

More than 100 times slower than that on Chrome 61.

Observe until the release.

t2ym commented 7 years ago

Suspected Root Cause - this is not the root cause

t2ym commented 7 years ago

Observation (Chrome Canary 63 only)

Verification Code

Object.getOwnPropertyNames(window).filter(name => name.indexOf('_pp_') < 0).forEach((name) => {
    let start = Date.now(); let o = _global[name];
    for (let i = 0; i < 100000; i++) { _globalObjects.get(o) === name; }
    let end = Date.now();
    console.log('name: ' + (name + ' '.repeat(48)).substr(0, 48) + ' ' + (' '.repeat(16) + (new Intl.NumberFormat()).format(parseInt(100000 * 1000 / (end - start)))).substr(-16) + ' op/s');
})

function _globalObjectsGet(o) { let name = o instanceof Object ? o.name : undefined; if (o === _global[name]) { return name; } else { return _globalObjects.get(o); } }
Object.getOwnPropertyNames(window).filter(name => name.indexOf('_pp_') < 0).forEach((name) => {
    let start = Date.now(); let o = _global[name];
    for (let i = 0; i < 100000; i++) { _globalObjectsGet(o) === name; }
    let end = Date.now();
    console.log('name: ' + (name + ' '.repeat(48)).substr(0, 48) + ' ' + (' '.repeat(16) + (new Intl.NumberFormat()).format(parseInt(100000 * 1000 / (end - start)))).substr(-16) + ' op/s');
})
t2ym commented 7 years ago

Chromium Source Code Analysis

https://codereview.chromium.org/2958063002 https://github.com/v8/v8/commit/aba708a1464043c73e7267a2baef500c2c6c0cc9 https://github.com/v8/v8/commit/4e9e81454049b59b256a71a604a9c36a341f923e https://github.com/v8/v8/blame/31cde16eeeccad72239c3036c8547a44a33f75d8/src/builtins/builtins-collections-gen.cc#L683

https://bugs.chromium.org/p/v8/issues/detail?id=6571 https://docs.google.com/document/d/13z1fvRVpe_oEroplXEEX0a3WK94fhXorHjcOMsDmR-8/edit#

t2ym commented 7 years ago

Potential Workaround

This solution is useless since the workaround is slower...

  const isSlowMap = (() => {
    const __globalObjectsGet = (o) => { let name = o instanceof Object ? o.name : undefined; if (o === _global[name]) { return name; } else { return _globalObjects.get(o); } };
    let startMap = Date.now();
    for (let i = 0; i < 100000; i++) {
      _globalObjects.get(Object)
    }
    let endMap = Date.now();
    let startProperty = Date.now();
    for (let i = 0; i < 100000; i++) {
      __globalObjectsGet(Object)
    }
    let endProperty = Date.now();
    return (endMap - startMap) > (endProperty - startProperty);
  })();
  let isInGlobalObjectsGet = false; // guard for recursive infinite calls
  const _globalObjectsGet = function _globalObjectsGet(o) {
    let name;
    if (!isInGlobalObjectsGet && typeof o === 'function') {
      isInGlobalObjectsGet = true;
      name = o.name;
      if (name && _globalObjects.values()[name] && _global[name] === o && _global.hasOwnProperty(name)) {
        isInGlobalObjectsGet = false;
        return name;
      }
      isInGlobalObjectsGet = false;
    }
    return _globalObjects.get(o);
  }
  // Usage:
  name = isSlowMap ? _globalObjectsGet(ctor) : _globalObjects.get(ctor);
t2ym commented 7 years ago

More practical approach: This is practically effective.

  // Note: Expecting JIT compiler to expand this function inline
  // Note: _globalObjectsGet() is faster for these case-labeled objects than Map.get() for them
  const _globalObjectsGet = typeof window === 'object'
    ? function _globalObjectsGet(o) {
        switch (o) {
        case undefined: return 'undefined';
        case Object: return 'Object';
        case Array: return 'Array';
        case Function: return 'Function';
        case Math: return 'Math';
        case window: return 'window';
        case RegExp: return 'RegExp';
        default:
          return _globalObjects.get(o);
        }
      }
    : function _globalObjectsGet(o) {
        switch (o) {
        case undefined: return 'undefined';
        case Object: return 'Object';
        case Array: return 'Array';
        case Function: return 'Function';
        case Math: return 'Math';
        case RegExp: return 'RegExp';
        default:
          return _globalObjects.get(o);
        }
      };
t2ym commented 7 years ago

Benchmarks with hookBenchmark('__hook__acl')

Chrome version hookacl . (op/s) = (op/s) () (op/s) f (op/s)
61.0.3163.100 0.0.159 1,824,817 2,110,595 954,380 5,824,111
63.0.3239.0 0.0.159 642,756 644,870 315,547 11,494,252
61.0.3163.100 0.0.160 2,236,636 2,283,626 1,034,768 6,172,839
63.0.3239.100 0.0.160 2,123,593 2,382,086 1,034,447 10,695,187
t2ym commented 7 years ago

The root cause is not fixed. So Chrome status should be tracked.

t2ym commented 7 years ago

The root cause is fixed in Chrome Canary 64.0.3240.0

Note

Chrome 64.0.3240.0

console.time(); for (let i = 0; i < 1000000000; i++) { _globalObjects.get(Object) === 'Object'; } console.timeEnd()
default: 8611.14501953125ms
console.time(); for (let i = 0; i < 1000000000; i++) { _globalObjects.get(Object); } console.timeEnd()
default: 620.914794921875ms

Chrome 61.0.3163.100


console.time(); for (let i = 0; i < 1000000000; i++) { _globalObjects.get(Object) === 'Object'; } console.timeEnd()
default: 1392.01904296875ms
console.time(); for (let i = 0; i < 1000000000; i++) { _globalObjects.get(Object); } console.timeEnd()
default: 619.695068359375ms```
t2ym commented 7 years ago

If the root cause is fixed in the Chrome 62 release (not beta), the changes for the issue will be removed.