GeezerVison

new is unnecessary anymore since it forces a return of a new Uint8Array.

@JacksonTian can you address this?

I prefer use new with constructor.

I do, too. I think there's even a linter rule in place for it. Are you fine with it @trevnorris?

@silverwind The code works fine, but we're allocating an unnecessary object. Though I doubt that has an impact on performance either. Part of the point is that Buffer is not an actual constructor anymore. It's now more of a factory method. But again it's nothing serious or is a change that needs to be made.

The change to lib/buffer.js LGTM but I wonder if it makes a difference on the benchmark numbers when you hoist out the conditional like this:

if (conf.type === 'buffer')
  for (let i = 0; i < n * 1024; i++) Buffer.from(zeroBuffer);
else if (conf.type === 'string')
  for (let i = 0; i < n * 1024; i++) Buffer.from(zeroString);

Also, I'm not sure if crankshaft will constant-fold the expression n * 1024 away, you might want to check if caching it makes a difference.

I think it's unrelated. these code is running in difference process when benchmarking it.

Sorry if I'm being unclear. What I mean is that the overhead of the check and the multiplication may affect the benchmark results. They probably don't but it would be good to check.

You mean the conf.type === 'buffer' will be run much times？

在 2016年3月23日，下午9:53，Ben Noordhuis <[email protected] mailto:[email protected]> 写道：

In benchmark/buffers/buffer_zero.js #4414 (comment):

function main(conf) {
var n = +conf.n;
bench.start();
for (let i = 0; i < n * 1024; i++) {

• Buffer.from(zero);
• conf.type === 'buffer' ? Buffer.from(zeroBuffer) : Buffer.from(zeroString);
  Sorry if I'm being unclear. What I mean is that the overhead of the check and the multiplication may affect the benchmark results. They probably don't but it would be good to check.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub https://github.com/nodejs/node/pull/4414/files/03579ba47a711923c67215d63385b60cdf473c98#r57161722