Skip to content

timers: clean up for readability #17279

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.

Sign up for GitHub

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

Closed
apapirovski wants to merge 3 commits into from
Closed

timers: clean up for readability #17279

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a77330e
timers: clean up for readability
apapirovski Nov 25, 2017
eff64e1
fixup: do not set domain in constructor
apapirovski Nov 26, 2017
4805558
fixup: address feedback on benchmark changes
apapirovski Nov 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
timers: clean up for readability 
Remove micro-optimizations that no longer yield any benefits,
restructure timers & immediates to be a bit more straightforward.

Adjust timers benchmarks to run long enough to offer meaningful data.
  • Loading branch information
@apapirovski
apapirovski committed Nov 26, 2017
commit a77330e8970af659a182455dbbcaffdb3e3d3b91
4 changes: 3 additions & 1 deletion benchmark/timers/immediate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
const common = require('../common.js');

const bench = common.createBenchmark(main, {
thousands: [2000],
thousands: [5000],
type: ['depth', 'depth1', 'breadth', 'breadth1', 'breadth4', 'clear']
});

Expand Down Expand Up @@ -88,6 +88,7 @@ function breadth1(N) {

// concurrent setImmediate, 4 arguments
function breadth4(N) {
N /= 2;
var n = 0;
bench.start();
function cb(a1, a2, a3, a4) {
Expand All @@ -101,6 +102,7 @@ function breadth4(N) {
}

function clear(N) {
N *= 4;
bench.start();
function cb(a1) {
if (a1 === 2)
Expand Down
4 changes: 2 additions & 2 deletions benchmark/timers/set-immediate-breadth-args.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ function main(conf) {
bench.start();
for (let i = 0; i < N; i++) {
if (i % 3 === 0)
setImmediate(cb3, 512, true, null);
setImmediate(cb3, 512, true, null, 512, true, null);
else if (i % 2 === 0)
setImmediate(cb2, false, 5.1);
setImmediate(cb2, false, 5.1, 512);
else
setImmediate(cb1, 0);
}
Expand Down
14 changes: 7 additions & 7 deletions benchmark/timers/set-immediate-depth-args.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

const common = require('../common.js');
const bench = common.createBenchmark(main, {
millions: [10]
millions: [5]
});

function main(conf) {
Expand All @@ -15,29 +15,29 @@ function main(conf) {
function cb3(n, arg2, arg3) {
if (--n) {
if (n % 3 === 0)
setImmediate(cb3, n, true, null);
setImmediate(cb3, n, true, null, 5.1, null, true);
else if (n % 2 === 0)
setImmediate(cb2, n, 5.1);
setImmediate(cb2, n, 5.1, true);
else
setImmediate(cb1, n);
}
}
function cb2(n, arg2) {
if (--n) {
if (n % 3 === 0)
setImmediate(cb3, n, true, null);
setImmediate(cb3, n, true, null, 5.1, null, true);
else if (n % 2 === 0)
setImmediate(cb2, n, 5.1);
setImmediate(cb2, n, 5.1, true);
else
setImmediate(cb1, n);
}
}
function cb1(n) {
if (--n) {
if (n % 3 === 0)
setImmediate(cb3, n, true, null);
setImmediate(cb3, n, true, null, 5.1, null, true);
else if (n % 2 === 0)
setImmediate(cb2, n, 5.1);
setImmediate(cb2, n, 5.1, true);
else
setImmediate(cb1, n);
}
Expand Down
2 changes: 1 addition & 1 deletion benchmark/timers/timers-breadth.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
const common = require('../common.js');

const bench = common.createBenchmark(main, {
thousands: [500],
thousands: [5000],
});

function main(conf) {
Expand Down
6 changes: 3 additions & 3 deletions benchmark/timers/timers-cancel-pooled.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ const common = require('../common.js');
const assert = require('assert');

const bench = common.createBenchmark(main, {
thousands: [500],
millions: [5],
});

function main(conf) {
const iterations = +conf.thousands * 1e3;
const iterations = +conf.millions * 1e6;

var timer = setTimeout(() => {}, 1);
for (var i = 0; i < iterations; i++) {
Expand All @@ -24,7 +24,7 @@ function main(conf) {
clearTimeout(timer);
}

bench.end(iterations / 1e3);
bench.end(iterations / 1e6);
}

function cb() {
Expand Down
6 changes: 3 additions & 3 deletions benchmark/timers/timers-cancel-unpooled.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ const common = require('../common.js');
const assert = require('assert');

const bench = common.createBenchmark(main, {
thousands: [100],
millions: [1],
});

function main(conf) {
const iterations = +conf.thousands * 1e3;
const iterations = +conf.millions * 1e6;

const timersList = [];
for (var i = 0; i < iterations; i++) {
Expand All @@ -18,7 +18,7 @@ function main(conf) {
for (var j = 0; j < iterations + 1; j++) {
clearTimeout(timersList[j]);
}
bench.end(iterations / 1e3);
bench.end(iterations / 1e6);
}

function cb() {
Expand Down
6 changes: 3 additions & 3 deletions benchmark/timers/timers-insert-pooled.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@
const common = require('../common.js');

const bench = common.createBenchmark(main, {
thousands: [500],
millions: [5],
});

function main(conf) {
const iterations = +conf.thousands * 1e3;
const iterations = +conf.millions * 1e6;

bench.start();

for (var i = 0; i < iterations; i++) {
setTimeout(() => {}, 1);
}

bench.end(iterations / 1e3);
bench.end(iterations / 1e6);
}
6 changes: 3 additions & 3 deletions benchmark/timers/timers-insert-unpooled.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,19 @@ const common = require('../common.js');
const assert = require('assert');

const bench = common.createBenchmark(main, {
thousands: [100],
millions: [1],
});

function main(conf) {
const iterations = +conf.thousands * 1e3;
const iterations = +conf.millions * 1e6;

const timersList = [];

bench.start();
for (var i = 0; i < iterations; i++) {
timersList.push(setTimeout(cb, i + 1));
}
bench.end(iterations / 1e3);
bench.end(iterations / 1e6);

for (var j = 0; j < iterations + 1; j++) {
clearTimeout(timersList[j]);
Expand Down
24 changes: 14 additions & 10 deletions benchmark/timers/timers-timeout-pooled.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,26 @@
const common = require('../common.js');

const bench = common.createBenchmark(main, {
thousands: [500],
millions: [10],
});

function main(conf) {
const iterations = +conf.thousands * 1e3;
var count = 0;
const iterations = +conf.millions * 1e6;

// Function tracking on the hidden class in V8 can cause misleading
// results in this benchmark if only a single function is used —
// alternate between two functions for a fairer benchmark

function cb() {}
function cb2() {}

process.on('exit', function() {
bench.end(iterations / 1e6);
Copy link
Copy Markdown
Contributor

@refack refack Nov 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not loving this... It's async, so it could add noise, also having the callbacks have side effects improves the chances they are not optimized away.
I'd rather have duplicate cb() code, or have cb just count++ and cb2 test for exit.

Copy link
Copy Markdown
Contributor Author

@apapirovski apapirovski Nov 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed with duplicate code.

});

for (var i = 0; i < iterations; i++) {
setTimeout(cb, 1);
setTimeout(i % 2 ? cb : cb2, 1);
Copy link
Copy Markdown
Contributor

@refack refack Nov 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: add a comment that this sets up n*1E6 timeout objects which will all get called together on next uv tick.
Which BTW mean this mesures:

  1. setup n*1E6 timeouts
  2. end tick - exit JS
  3. poll in uv for 1ms
  4. reenter JS
  5. call n*1E6 callbacks

I wish we could exclude steps 2-4 from the measurement 🤔

Copy link
Copy Markdown
Contributor Author

@apapirovski apapirovski Nov 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment.

}

bench.start();

function cb() {
count++;
if (count === iterations)
bench.end(iterations / 1e3);
}
}
Loading