buffer: Fixing deopt when constructed with strings

[tomgco]

When a buffer was created using new Buffer('A String'), internally
within the buffer class we would read arguments[1], however in this case
we would be reading outside of the array causing a deopt to occur.

This fixes this issue by casting the type to a string before use, and
removing the usage of the arguments object.

[evanlucas]

/cc @trevnorris

[evanlucas]

This seems to decrease performance of buffer creation for fast cases with a larger length pretty consistently...

[~/dev/code/forks/node] :] (v5.0.0) ➜ $ ./node benchmark/compare.js ./node /usr/local/bin/node -- buffers buffer-creation (tmp2) running ./node buffers/buffer-creation.js running /usr/local/bin/node buffers/buffer-creation.js buffers/buffer-creation.js type=fast len=10 n=1024: ./node: 4696.1 /usr/local/bin/node: 4510.4 ..... 4.12% buffers/buffer-creation.js type=fast len=1024 n=1024: ./node: 1873.5 /usr/local/bin/node: 2602.1 . -28.00% buffers/buffer-creation.js type=slow len=10 n=1024: ./node: 1510.6 /usr/local/bin/node: 1495 ....... 1.05% buffers/buffer-creation.js type=slow len=1024 n=1024: ./node: 1345.4 /usr/local/bin/node: 1376.8 .. -2.28% [~/dev/code/forks/node] :] (v5.0.0) ➜ $ ./node benchmark/compare.js ./node /usr/local/bin/node -- buffers buffer-creation (tmp2) running ./node buffers/buffer-creation.js running /usr/local/bin/node buffers/buffer-creation.js buffers/buffer-creation.js type=fast len=10 n=1024: ./node: 4713.8 /usr/local/bin/node: 4370 ..... 7.87% buffers/buffer-creation.js type=fast len=1024 n=1024: ./node: 1886 /usr/local/bin/node: 2721.7 . -30.71% buffers/buffer-creation.js type=slow len=10 n=1024: ./node: 1505.3 /usr/local/bin/node: 1479 ..... 1.78% buffers/buffer-creation.js type=slow len=1024 n=1024: ./node: 1395.4 /usr/local/bin/node: 1350.6 . 3.32% [~/dev/code/forks/node] :] (v5.0.0) ➜ $ ./node benchmark/compare.js ./node /usr/local/bin/node -- buffers buffer-creation (tmp2) running ./node buffers/buffer-creation.js running /usr/local/bin/node buffers/buffer-creation.js buffers/buffer-creation.js type=fast len=10 n=1024: ./node: 4732.1 /usr/local/bin/node: 4518.9 ..... 4.72% buffers/buffer-creation.js type=fast len=1024 n=1024: ./node: 1869.5 /usr/local/bin/node: 2838.9 . -34.15% buffers/buffer-creation.js type=slow len=10 n=1024: ./node: 1526.3 /usr/local/bin/node: 1497.2 ..... 1.94% buffers/buffer-creation.js type=slow len=1024 n=1024: ./node: 1408.4 /usr/local/bin/node: 1386.2 ... 1.60% [~/dev/code/forks/node] :] (v5.0.0) ➜ $ ./node benchmark/compare.js ./node /usr/local/bin/node -- buffers buffer-creation (tmp2) running ./node buffers/buffer-creation.js running /usr/local/bin/node buffers/buffer-creation.js buffers/buffer-creation.js type=fast len=10 n=1024: ./node: 4707.8 /usr/local/bin/node: 4518.7 ..... 4.18% buffers/buffer-creation.js type=fast len=1024 n=1024: ./node: 1934.2 /usr/local/bin/node: 2684.9 . -27.96% buffers/buffer-creation.js type=slow len=10 n=1024: ./node: 1483.8 /usr/local/bin/node: 1505.7 .... -1.45% buffers/buffer-creation.js type=slow len=1024 n=1024: ./node: 1349.4 /usr/local/bin/node: 1345.3 ... 0.30% 

[tomgco]

This seems to decrease performance of buffer creation for fast cases with a larger length pretty consistently...

Thanks, I wasn't aware of benchmark/compare.js, I will have a look to see why that is the case, thanks!

[tomgco]

@evanlucas I have been running benchmarks against this, however I am unable to replicate.

I am, however getting a high margin of slow on multiple tests for two identical versions of node, node 5.1.0 vs 5.1.0 even show the same discrepancies. (up to 10-5%)

However even with that, my changes seem to be consistently positive, with a higher number of operations.

[tomgco]

Note, I have am using a patched version of the benchmarking tools to run all processes at a niceness value of -20 (highest priority).

tomgco@King ~/Projects/contrib/node (master●)$ sudo nice -n -20 ./node benchmark/compare.js ../node-01d2798 ./node -- buffers buffer-creation running ../node-01d2798 buffers/buffer-creation.js running ./node buffers/buffer-creation.js buffers/buffer-creation.js type=fast len=10 n=1024: ../node-01d2798: 2771.4 ./node: 2707.5 ... 2.36% buffers/buffer-creation.js type=fast len=1024 n=1024: ../node-01d2798: 1669.8 ./node: 1599.6 . 4.39% buffers/buffer-creation.js type=slow len=10 n=1024: ../node-01d2798: 1598.6 ./node: 1452.6 .. 10.05% buffers/buffer-creation.js type=slow len=1024 n=1024: ../node-01d2798: 1212.2 ./node: 1178.4 . 2.86% tomgco@King ~/Projects/contrib/node (master●)$ sudo nice -n -20 ./node benchmark/compare.js ../node-01d2798 ./node -- buffers buffer-creation running ../node-01d2798 buffers/buffer-creation.js running ./node buffers/buffer-creation.js buffers/buffer-creation.js type=fast len=10 n=1024: ../node-01d2798: 2757.7 ./node: 2719.7 ... 1.40% buffers/buffer-creation.js type=fast len=1024 n=1024: ../node-01d2798: 1672.1 ./node: 1594.1 . 4.89% buffers/buffer-creation.js type=slow len=10 n=1024: ../node-01d2798: 1617.9 ./node: 1546.9 ... 4.59% buffers/buffer-creation.js type=slow len=1024 n=1024: ../node-01d2798: 1192 ./node: 1136.2 ... 4.91% tomgco@King ~/Projects/contrib/node (master●)$ sudo nice -n -20 ./node benchmark/compare.js ../node-01d2798 ./node -- buffers buffer-creation running ../node-01d2798 buffers/buffer-creation.js running ./node buffers/buffer-creation.js buffers/buffer-creation.js type=fast len=10 n=1024: ../node-01d2798: 2769.4 ./node: 2710.8 ... 2.16% buffers/buffer-creation.js type=fast len=1024 n=1024: ../node-01d2798: 1690.8 ./node: 1603.1 . 5.47% buffers/buffer-creation.js type=slow len=10 n=1024: ../node-01d2798: 1509.9 ./node: 1541.1 .. -2.02% buffers/buffer-creation.js type=slow len=1024 n=1024: ../node-01d2798: 1171.5 ./node: 1145.3 . 2.29% tomgco@King ~/Projects/contrib/node (master●)$ sudo nice -n -20 ./node benchmark/compare.js ../node-01d2798 ./node -- buffers buffer-creation running ../node-01d2798 buffers/buffer-creation.js running ./node buffers/buffer-creation.js buffers/buffer-creation.js type=fast len=10 n=1024: ../node-01d2798: 2743.8 ./node: 2710.5 ... 1.23% buffers/buffer-creation.js type=fast len=1024 n=1024: ../node-01d2798: 1658.1 ./node: 1606.7 . 3.20% buffers/buffer-creation.js type=slow len=10 n=1024: ../node-01d2798: 1614.5 ./node: 1448.5 .. 11.45% buffers/buffer-creation.js type=slow len=1024 n=1024: ../node-01d2798: 1255.1 ./node: 1183.7 . 6.03% tomgco@King ~/Projects/contrib/node (master●)$ sudo nice -n -20 ./node benchmark/compare.js ../node-01d2798 ./node -- buffers buffer-creation running ../node-01d2798 buffers/buffer-creation.js running ./node buffers/buffer-creation.js buffers/buffer-creation.js type=fast len=10 n=1024: ../node-01d2798: 2746.8 ./node: 2704.8 ... 1.55% buffers/buffer-creation.js type=fast len=1024 n=1024: ../node-01d2798: 1639.9 ./node: 1603.9 . 2.24% buffers/buffer-creation.js type=slow len=10 n=1024: ../node-01d2798: 1617.6 ./node: 1550.5 ... 4.33% buffers/buffer-creation.js type=slow len=1024 n=1024: ../node-01d2798: 1200.5 ./node: 1170.6 . 2.55%

node-01d2798 being this PR and node being master.

[tomgco]

For clarity, this is what I get when I run master vs master, with the ./node symlinked to ./node-2.

As you can see the tests seem to like to deviate even on identical binarys.

Now this could be due to kernel settings, the cpu scheduler, etc, I will investigate more into this.

tomgco@King ~/Projects/contrib/node (benchmark-updates●●)$ ./node benchmark/compare.js ./node ./node-2 -- buffers buffer-creation running ./node buffers/buffer-creation.js running ./node-2 buffers/buffer-creation.js buffers/buffer-creation.js type=fast len=10 n=1024: ./node: 2697.2 ./node-2: 2708.6 ... -0.42% buffers/buffer-creation.js type=fast len=1024 n=1024: ./node: 1600.3 ./node-2: 1605.4 . -0.32% buffers/buffer-creation.js type=slow len=10 n=1024: ./node: 1545 ./node-2: 1448.1 ...... 6.69% buffers/buffer-creation.js type=slow len=1024 n=1024: ./node: 1140.1 ./node-2: 1139.9 .. 0.01% tomgco@King ~/Projects/contrib/node (benchmark-updates●●)$ ./node benchmark/compare.js ./node ./node-2 -- buffers buffer-creation running ./node buffers/buffer-creation.js running ./node-2 buffers/buffer-creation.js buffers/buffer-creation.js type=fast len=10 n=1024: ./node: 2708.2 ./node-2: 2703.1 .... 0.19% buffers/buffer-creation.js type=fast len=1024 n=1024: ./node: 1598.9 ./node-2: 1596.4 .. 0.16% buffers/buffer-creation.js type=slow len=10 n=1024: ./node: 1457.5 ./node-2: 1543.8 ... -5.59% buffers/buffer-creation.js type=slow len=1024 n=1024: ./node: 1115.9 ./node-2: 1141.9 . -2.28% tomgco@King ~/Projects/contrib/node (benchmark-updates●●)$ ./node benchmark/compare.js ./node ./node-2 -- buffers buffer-creation running ./node buffers/buffer-creation.js running ./node-2 buffers/buffer-creation.js buffers/buffer-creation.js type=fast len=10 n=1024: ./node: 2729.6 ./node-2: 2708.8 ... 0.77% buffers/buffer-creation.js type=fast len=1024 n=1024: ./node: 1601.1 ./node-2: 1594.4 . 0.42% buffers/buffer-creation.js type=slow len=10 n=1024: ./node: 1325.7 ./node-2: 1545.1 . -14.20% buffers/buffer-creation.js type=slow len=1024 n=1024: ./node: 1107.4 ./node-2: 1105.6 . 0.16% 

[trevnorris]

@tomgco Those benchmarks need to be run at least a dozen times in order to get a good median result for comparison. Here's a one-liner:

$ for i in `seq 12`; do node ./buffer-creation.js type=fast len=1024 n=1024 | cut -d : -f 2 | sed -e 's/^[[:space:]]*//'; done 

[tomgco]

@trevnorris lovely, would it be worth incorporating that into the tests?

[trevnorris]

@tomgco I like that idea, but also think it should be done in a different way for all the benchmarks. i.e. the current benchmark package needs to be rewritten.

[tomgco]

@trevnorris I am going to close this issue as I am unable to create a reproducable benchmark.