Skip to content
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.

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

Add benchmarks #101

Merged
merged 8 commits into from Aug 23, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
70 changes: 70 additions & 0 deletions benchmark.js
@@ -0,0 +1,70 @@
'use strict';

const path = require('path');
const Benchmark = require('benchmark');
const makeDir = require('make-dir');
const tempy = require('tempy');
const del = require('.');

const suite = new Benchmark.Suite('concurrency');

const tempDir = tempy.directory();

const fixtures = Array.from({length: 2000}, (x, index) => {
return path.resolve(tempDir, (index + 1).toString());
});

function createFixtures() {
for (const fixture of fixtures) {
makeDir.sync(path.resolve(tempDir, fixture));
}
}

const concurrency = [1, 3, 5, 10, 15, 20, 50, 100, Infinity];

concurrency.forEach(num => {
chrisblossom marked this conversation as resolved.
Show resolved Hide resolved
const name = `concurrency: ${num.toString()}`;

suite.add({
name,
defer: true,
fn(deferred) {
// Can't use setup because it isn't called after every defer
// https://github.com/bestiejs/benchmark.js/issues/136
sindresorhus marked this conversation as resolved.
Show resolved Hide resolved
createFixtures();

// Async await was giving too many errors. stick with standard promises
Copy link
Owner

Choose a reason for hiding this comment

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

Huh? What does this mean? Async/await shouldn't work any differently.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

suite.add({
	name,
	defer: true,
    // if you remove setup() the benchmark works as expected
	setup() {
	},
	async fn(deferred) {
		// Can't use setup because it isn't called after every defer
		// https://github.com/bestiejs/benchmark.js/issues/136
		createFixtures();

		// Async await was giving too many errors. stick with standard promises
		await del(['**/*'], {
			cwd: tempDir,
			concurrency,
			// eslint-disable-next-line promise/prefer-await-to-then
		}).then(removedFiles => {
			if (removedFiles.length !== fixtures.length) {
				const error = new Error(
					`"${name}": files removed: ${removedFiles.length}, expected: ${fixtures.length}`,
				);

				console.error(error);

				del.sync(tempDir, { cwd: tempDir, force: true });

				// eslint-disable-next-line unicorn/no-process-exit
				process.exit(1);
			}

			deferred.resolve();
		});
	},
});

Gives the error:

del on  add-benchmarks [⇡!] is 📦 v5.0.0 via ⬢ v10.16.0 took 6s
➜ node benchmark.js
undefined:9
			await del(['**/*'], {
			^^^^^

SyntaxError: await is only valid in async function
    at Function (<anonymous>)
    at createCompiled (/Users/chris/github/del/node_modules/benchmark/benchmark.js:1725:16)
    at clock (/Users/chris/github/del/node_modules/benchmark/benchmark.js:1608:58)
    at clock (/Users/chris/github/del/node_modules/benchmark/benchmark.js:1818:20)
    at new Deferred (/Users/chris/github/del/node_modules/benchmark/benchmark.js:405:7)
    at Deferred (/Users/chris/github/del/node_modules/benchmark/benchmark.js:402:16)
    at Benchmark.run (/Users/chris/github/del/node_modules/benchmark/benchmark.js:2112:13)
    at execute (/Users/chris/github/del/node_modules/benchmark/benchmark.js:860:74)
    at Timeout._onTimeout (/Users/chris/github/del/node_modules/lodash/lodash.js:2750:43)
    at ontimeout (timers.js:436:11)

Copy link
Owner

Choose a reason for hiding this comment

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

Weird. I already commented on this, but it's gone now... Anyway, I think you're just missing the async keyword for the method. Then it worked for me at least.

del(['**/*'], {
cwd: tempDir,
concurrency: num
// eslint-disable-next-line promise/prefer-await-to-then
}).then(removedFiles => {
sindresorhus marked this conversation as resolved.
Show resolved Hide resolved
if (removedFiles.length !== fixtures.length) {
const error = new Error(
`"${name}": files removed: ${removedFiles.length}, expected: ${fixtures.length}`,
);

console.error(error);

del.sync(tempDir, {cwd: tempDir, force: true});

// eslint-disable-next-line unicorn/no-process-exit
process.exit(1);
}

deferred.resolve();
});
}
});
});

suite
.on('cycle', event => {
console.log(String(event.target));
sindresorhus marked this conversation as resolved.
Show resolved Hide resolved
})
.on('complete', function () {
console.log(`Fastest is ${this.filter('fastest').map('name')}`);

del.sync(tempDir, {cwd: tempDir, force: true});
})
.run({async: true});
4 changes: 3 additions & 1 deletion package.json
Expand Up @@ -13,7 +13,8 @@
"node": ">=8"
},
"scripts": {
"test": "xo && ava && tsd"
"test": "xo && ava && tsd",
"bench": "node benchmark.js"
},
"files": [
"index.js",
Expand Down Expand Up @@ -52,6 +53,7 @@
},
"devDependencies": {
"ava": "^2.1.0",
"benchmark": "^2.1.4",
"make-dir": "^3.0.0",
"tempy": "^0.3.0",
"tsd": "^0.7.3",
Expand Down