| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
I'm not sure it's worth removing. Benchmarks like this probably should have defined parameters containing defaults of at least the currently used values. |
Sorry, something went wrong.
@mscdex Does this mean you would endorse (or at least not object to) a change from this: var bench = common.createBenchmark(main, {});
function main(conf) {
var N = 64 * 1024 * 1024;
var b = Buffer.allocUnsafe(N);
var s = '';
var i;
for (i = 0; i < 256; ++i) s += String.fromCharCode(i);
for (i = 0; i < N; i += 256) b.write(s, i, 256, 'ascii');
bench.start();
for (i = 0; i < 32; ++i) b.toString('base64');
bench.end(64);
}...to something more like this?: var bench = common.createBenchmark(main, {
N: [64 * 1024 * 1024],
});
function main(conf) {
var N = +conf.N;
var b = Buffer.allocUnsafe(N);
var s = '';
var i;
for (i = 0; i < 256; ++i) s += String.fromCharCode(i);
for (i = 0; i < N; i += 256) b.write(s, i, 256, 'ascii');
bench.start();
for (i = 0; i < 32; ++i) b.toString('base64');
bench.end(64);
} |
Sorry, something went wrong.
|
@Trott Pretty much, although the iteration count might be configurable as well like with most other benchmarks. |
Sorry, something went wrong.
|
@troy0820 I think it would be better, yes. Just make sure to also add a parameter for the number of iterations (usually lowercase n). |
Sorry, something went wrong.
@mscdex That would be the number of iterations in the loop between bench.start() and bench.end(), right? So, in the above example, it's 32? Also, would that value be passed to bench.end()? In the example above, 64 is hard-coded as what is sent to bench.end(), so... |
Sorry, something went wrong.
|
@Trott Correct. I don't know why 64 was being passed in the encode benchmark, it should be the loop count which is 32. |
Sorry, something went wrong.
|
@troy0820 You up for making those changes as described above and trying to ascertain what the similar changes might be for the other file? (I'd guess just the iterations and nothing else in that file, but take a look and judge for yourself.) |
Sorry, something went wrong.
|
@Trott Yeah I can make those changes. It sounds like it's the above snippet along with the only change being the 32 in the bench.end(32) function and adding then param to the main function. |
Sorry, something went wrong.
|
@troy0820 You'll want to add an n variable to the conf for both files and set it to 32. Then you'll want to assign it to a variable (like I did in the snippet with N) and use that variable in the loop and in bench.end(). Hit me up on IRC if this isn't clear, or just make a go at it and we'll iterate if it's not quite right. |
Sorry, something went wrong.
There was a problem hiding this comment.
We might want to give this a better name to avoid confusion. I think other benchmarks use names like len, which I would be fine with.
Sorry, something went wrong.
There was a problem hiding this comment.
This should be +conf.n.
Sorry, something went wrong.
There was a problem hiding this comment.
Similarly for these, the values should be prefixed with + to ensure they are numbers.
Sorry, something went wrong.
There was a problem hiding this comment.
Sorry, I meant this parameter should be called len. n should stay the same.
Sorry, something went wrong.
There was a problem hiding this comment.
Is the change from 64 to 32 in the bench.end(n) here intentional?
Sorry, something went wrong.
There was a problem hiding this comment.
Nevermind... just spotted the other comment about it :-)
Sorry, something went wrong.
There was a problem hiding this comment.
This should be const n = +conf.n;
Sorry, something went wrong.
There was a problem hiding this comment.
This should be const len = +conf.len;
Sorry, something went wrong.
There was a problem hiding this comment.
This should be const b = Buffer.allocUnsafe(len);
Sorry, something went wrong.
There was a problem hiding this comment.
N here should be len
Sorry, something went wrong.
|
Rebased master and made changes @mscdex |
Sorry, something went wrong.
|
LGTM |
Sorry, something went wrong.
|
Only relevant CI job for this is the linter, so here it is: |
Sorry, something went wrong.
…on in buffer-base64-encode & buffer-base64-decode.js
|
Hmmm, the linter failed, but that might have been because it needed a rebase, which I just did. Let's try again... |
Sorry, something went wrong.
|
Linter is ✅ |
Sorry, something went wrong.
Add configuration object createBenchmark object for buffer size & iteration in buffer-base64-encode & buffer-base64-decode.js. PR-URL: nodejs#10175 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net>
Sorry, something went wrong.
Add configuration object createBenchmark object for buffer size & iteration in buffer-base64-encode & buffer-base64-decode.js. PR-URL: #10175 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net>
Add configuration object createBenchmark object for buffer size & iteration in buffer-base64-encode & buffer-base64-decode.js. PR-URL: #10175 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net>
Add configuration object createBenchmark object for buffer size & iteration in buffer-base64-encode & buffer-base64-decode.js. PR-URL: #10175 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net>
Add configuration object createBenchmark object for buffer size & iteration in buffer-base64-encode & buffer-base64-decode.js. PR-URL: #10175 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net>
Add configuration object createBenchmark object for buffer size & iteration in buffer-base64-encode & buffer-base64-decode.js. PR-URL: #10175 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net>
| Back | FazBrowse Home | New Git URL |
Removed unused variable conf from buffer-base64-decode.js and buffer-based64-encode.js
Checklist
Affected core subsystem(s)
Buffer
Description of change
Deleted variable that wasn't used which was conf