{"action":"created","issue":{"url":"https://api.github.com/repos/nodejs/benchmarking/issues/38","repository_url":"https://api.github.com/repos/nodejs/benchmarking","labels_url":"https://api.github.com/repos/nodejs/benchmarking/issues/38/labels{/name}","comments_url":"https://api.github.com/repos/nodejs/benchmarking/issues/38/comments","events_url":"https://api.github.com/repos/nodejs/benchmarking/issues/38/events","html_url":"https://github.com/nodejs/benchmarking/issues/38","id":141807604,"number":38,"title":"Regressions in buffer operations - 4.4.0","user":{"login":"gareth-ellis","id":14981026,"avatar_url":"https://avatars.githubusercontent.com/u/14981026?v=3","gravatar_id":"","url":"https://api.github.com/users/gareth-ellis","html_url":"https://github.com/gareth-ellis","followers_url":"https://api.github.com/users/gareth-ellis/followers","following_url":"https://api.github.com/users/gareth-ellis/following{/other_user}","gists_url":"https://api.github.com/users/gareth-ellis/gists{/gist_id}","starred_url":"https://api.github.com/users/gareth-ellis/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gareth-ellis/subscriptions","organizations_url":"https://api.github.com/users/gareth-ellis/orgs","repos_url":"https://api.github.com/users/gareth-ellis/repos","events_url":"https://api.github.com/users/gareth-ellis/events{/privacy}","received_events_url":"https://api.github.com/users/gareth-ellis/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":4,"created_at":"2016-03-18T08:50:42Z","updated_at":"2016-03-18T18:05:52Z","closed_at":null,"body":"Hello,\r\n\r\nOne of the benchmarks we run contains what is in effect a number of microbenchmarks. We do a number of different actions with buffers, and I noticed in 4.4.0 we have seen a drop in some of these buffer tests. \r\nOne of these tests is looking at how long it takes to create a new buffer from an array of data.\r\n```js\r\nvar harness = require('./harness.js');\r\n\r\nvar ARRAY = [1, 2, 23829, 4, 5, 7, 12312321, 2131, 434832, 43792, 23421, 65345, 132210, 77777, 322131, 1, 2, 23829, 4, 5, 7, 12312321, 2131, 434832, 43792, 23421, 65345, 132210, 77777, 322131, 1, 2, 23829, 4, 5, 7, 12312321, 2131, 434832, 43792, 23421, 65345, 132210, 77777, 322131, 1, 2, 23829, 4, 5, 7, 12312321, 2131, 434832, 43792, 23421, 65345, 132210, 77777, 322131];\r\nvar ITERATIONS = 300000;\r\nvar result;\r\n\r\nfunction test() {\r\n\tfor(var i=0;i<ITERATIONS;i++) {\r\n\t\tresult = new Buffer(ARRAY);\r\n\t}\r\n}\r\nharness.run_test(test);\r\n```\r\n\r\nOur harness repeats the test function a number of times, either until we get good consistency in scores, or until a max number of attempts. We then report a number of operations per second.\r\n\r\nComparing 4.3.2 and 4.4.0 we saw quite a drop in the ops/sec for this test. Looking at the changes between the builds, I saw that 4.4.0 had some changes to lib/buffer.js as per https://github.com/nodejs/node/pull/4886 .\r\n\r\nI rebuilt node 4.4.0 with the buffer.js from 4.3.2 and saw the performance recover:\r\n\r\n**4.4.0 with the buffer.js from 4.3.2:**\r\nsdk4.4.0_oldbuffer/bin/node new-buffer-from-array-test.js \r\ntotal time: 5.093s -- iterations: 43 -- ops/sec: 8.44 -- average time: 0.12s -- variance: 2.19%\r\ntotal time: 5.077s -- iterations: 43 -- ops/sec: 8.47 -- average time: 0.12s -- variance: 0.22%\r\ntotal time: 5.076s -- iterations: 43 -- ops/sec: 8.47 -- average time: 0.12s -- variance: 0.32%\r\ntotal time: 5.076s -- iterations: 43 -- ops/sec: 8.47 -- average time: 0.12s -- variance: 0.18%\r\n\r\n**4.4.0 unmodified**\r\nsdk4.40/bin/node new-buffer-from-array-test.js \r\ntotal time: 5.003s -- iterations: 26 -- ops/sec: 5.2 -- average time: 0.19s -- variance: 4.42%\r\ntotal time: 5.059s -- iterations: 26 -- ops/sec: 5.14 -- average time: 0.19s -- variance: 7.04%\r\ntotal time: 5.149s -- iterations: 27 -- ops/sec: 5.24 -- average time: 0.19s -- variance: 0.24%\r\ntotal time: 5.149s -- iterations: 27 -- ops/sec: 5.24 -- average time: 0.19s -- variance: 0.24%\r\n\r\n\r\nLooking at the changes in buffer.js, they mainly seem to be changing var to either const or let.\r\n\r\nI reverted these ones, and saw the performance improve:\r\n\r\n**4.4.0 with the const & let changes switched back to var**\r\nsdk4.4.0-testfix/bin/node new-buffer-from-array-test.js \r\ntotal time: 5.062s -- iterations: 42 -- ops/sec: 8.3 -- average time: 0.12s -- variance: 2.11%\r\ntotal time: 5.039s -- iterations: 42 -- ops/sec: 8.33 -- average time: 0.12s -- variance: 0.13%\r\ntotal time: 5.045s -- iterations: 42 -- ops/sec: 8.33 -- average time: 0.12s -- variance: 1.09%\r\ntotal time: 5.039s -- iterations: 42 -- ops/sec: 8.33 -- average time: 0.12s -- variance: 0.29%\r\n\r\n\r\nGranted not quite to 100%, but much closer. Any ideas why this would cause a regression?"},"comment":{"url":"https://api.github.com/repos/nodejs/benchmarking/issues/comments/198475175","html_url":"https://github.com/nodejs/benchmarking/issues/38#issuecomment-198475175","issue_url":"https://api.github.com/repos/nodejs/benchmarking/issues/38","id":198475175,"user":{"login":"jasnell","id":439929,"avatar_url":"https://avatars.githubusercontent.com/u/439929?v=3","gravatar_id":"","url":"https://api.github.com/users/jasnell","html_url":"https://github.com/jasnell","followers_url":"https://api.github.com/users/jasnell/followers","following_url":"https://api.github.com/users/jasnell/following{/other_user}","gists_url":"https://api.github.com/users/jasnell/gists{/gist_id}","starred_url":"https://api.github.com/users/jasnell/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jasnell/subscriptions","organizations_url":"https://api.github.com/users/jasnell/orgs","repos_url":"https://api.github.com/users/jasnell/repos","events_url":"https://api.github.com/users/jasnell/events{/privacy}","received_events_url":"https://api.github.com/users/jasnell/received_events","type":"User","site_admin":false},"created_at":"2016-03-18T18:05:52Z","updated_at":"2016-03-18T18:05:52Z","body":"It's likely due to some point in time optimization quirk with the version of v8. We can likely get those `let`/`const` uses reverted in v4, just need a PR opened against v4.x-staging to do so!"}}