Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib/internal/util.js#join function is slower than the built-in Array#join #33732

Closed
sapics opened this issue Jun 5, 2020 · 3 comments
Closed

Comments

@sapics
Copy link
Contributor

sapics commented Jun 5, 2020

When I run the below code with v12.16.2 in Win10, the performance of lib/internal/util.js#join is about 30 times slower than native Array#join. It looks better to replace to native one.
This function is used in lib/internal/util/inspect.js in a quick look.
I cannot judge an argument output is Array or ArrayLike, so I could not fix, sorry.
(Add: This would be one of the ways to replace easily ref)

Join string performance check
native/join: 5.081699997186661 ms
  util/join: 146.29249899089336 ms

Join number performance check
native/join: 4.920000001788139 ms
  util/join: 155.2229000031948 ms
const {performance} = require('perf_hooks')

/* start copy from lib/internal/util.js #L326 */
// The build-in Array#join is slower in v8 6.0
function join(output, separator) {
  let str = '';
  if (output.length !== 0) {
    const lastIndex = output.length - 1;
    for (let i = 0; i < lastIndex; i++) {
      // It is faster not to use a template string here
      str += output[i];
      str += separator;
    }
    str += output[lastIndex];
  }
  return str;
}
/* end copy lib/internal/util.js #L339 */

const arr1 = (new Array(1000000)).fill(0).map((v,i) => i % 10 + "")
const arr2 = (new Array(1000000)).fill(0).map((v,i) => i % 10)
var time1, time2

console.log('Join string performance check')
time1 = performance.now()
arr1.join('@')
time2 = performance.now()
join(arr1, '@')
time3 = performance.now()
console.log('native/join:', time2-time1, 'ms')
console.log('  util/join:', time3-time2, 'ms')

console.log('\nJoin number performance check')
time1 = performance.now()
arr2.join('@')
time2 = performance.now()
join(arr2, '@')
time3 = performance.now()
console.log('native/join:', time2-time1, 'ms')
console.log('  util/join:', time3-time2, 'ms')
@bnoordhuis
Copy link
Member

I suppose it's not too surprising V8 caught up with us. :-)

A pull request switching to ArrayPrototypeJoin() is welcome. You can get it from the primordials internal object:

const { ArrayPrototypeJoin } = primordials;

That's Array#join() but copied out to be tamper-proof against prototype monkey-patching.

@BridgeAR
Copy link
Member

BridgeAR commented Jun 5, 2020

The benchmark will only be faster for big arrays. Small arrays still profit from the current implementation and that's why it's currently used.

@sapics
Copy link
Contributor Author

sapics commented Jun 5, 2020

@bnoordhuis Thank you for your advice!
It looks better than mine. I made a PR in #33737, but I close it as below.

@BridgeAR Thank you for teaching the reason to apply the function!
I simulated similar one with smaller array, and I could confirm your point.
Thus, I close the PR #33737.

Join string performance check
native/join: 16.52709999680519
  util/join: 9.16829900443554

Join number performance check
native/join: 14.551398992538452
  util/join: 9.72800000011921
const {performance} = require('perf_hooks')

/* start from lib/internal/util.js #L326 */
// The build-in Array#join is slower in v8 6.0
function join(output, separator) {
  let str = '';
  if (output.length !== 0) {
    const lastIndex = output.length - 1;
    for (let i = 0; i < lastIndex; i++) {
      // It is faster not to use a template string here
      str += output[i];
      str += separator;
    }
    str += output[lastIndex];
  }
  return str;
}
/* end at lib/internal/util.js #L339 */

const arr1 = (new Array(5)).fill(0).map((v,i) => i % 10 + "")
const arr2 = (new Array(5)).fill(0).map((v,i) => i % 10)
var time1, time2

console.log('Join string performance check')
time1 = performance.now()
for(var i = 0; i < 100000; ++i) arr1.join('@')
time2 = performance.now()
for(var i = 0; i < 100000; ++i) join(arr1, '@')
time3 = performance.now()
console.log('native/join:', time2-time1)
console.log('  util/join:', time3-time2)

console.log('\nJoin number performance check')
time1 = performance.now()
for(var i = 0; i < 100000; ++i) arr2.join('@')
time2 = performance.now()
for(var i = 0; i < 100000; ++i) join(arr2, '@')
time3 = performance.now()
console.log('native/join:', time2-time1)
console.log('  util/join:', time3-time2)

@sapics sapics closed this as completed Jun 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants