Skip to content

Conversation

@jamestalmage
Copy link
Contributor

I want to start focusing on getting our require times down,
both in the main thread and forked tests.

This adds time-require support directly to the CLI to assist in that.
This is especially important for forked test processes, where I currently
have to manually add a line of code each time I want to test an optimization.

I don't think this should be a publicly documented move. I think we want
the freedom to remove this once we are happy with performance.

I have placed comments above all time-require references, indicating
it is for internal use only. Users have been given fair warning against
relying on its inclusion long term.

Usage:

$ ./cli.js test/fixture/es2015.js --time-require --sorted

--sorted is optional. It sorts the requires from most expensive to least. The default is sequential sorting.

The output will display multiple time-require reports.
One for each forked process, and the last one for the main thread.

@sindresorhus
Copy link
Member

Can't we just use debug for this instead of a separate flag?

@jamestalmage
Copy link
Contributor Author

I am not sure I understand how to use the debug module to accomplish that.

This way time-require is not setting it's require-hook unless the flag is set.

@sindresorhus
Copy link
Member

var debug = require('debug')('ava');

if (debug.enabled) {
  require('time-require');
}

@jamestalmage
Copy link
Contributor Author

Got it. That works.

How should I handle --sorted vs not? It is nice to have both, but not required.

If I have to pick one, then I guess I'd prefer the default sequential sorting (easier to figure out which module is requiring acorn that way).

@sindresorhus
Copy link
Member

How should I handle --sorted vs not? It is nice to have both, but not required.

If you feel it would be useful, just forward it to the forked processes like we do with the other ones.

@sindresorhus
Copy link
Member

I want to start focusing on getting our require times down,
both in the main thread and forked tests.

This adds `time-require` support directly to the CLI to assist in that.
This is especially important for forked test processes, where I currently
have to manually add a line of code each time I want to test an optimization.

I don't think this should be a publicly documented move. I think we want
the freedom to remove this once we are happy with performance.

I have placed comments above all `time-require` references, indicating
it is for internal use only. Users have been given fair warning against
relying on its inclusion long term.

Usage:

```sh
$ DEBUG=ava ./cli.js test/fixture/es2015.js --sorted
```

`--sorted` is optional. It sorts the requires from most expensive to least. The default is sequential sorting.

The output will display multiple `time-require` reports.
One for each forked process, and the last one for the main thread.
@jamestalmage
Copy link
Contributor Author

I just checked out both of those. They are a lot more verbose and don't provide the sort option. time-require provides nicer output, especially once #275 gets merged.

@jamestalmage
Copy link
Contributor Author

I think AppVeyor ate too much turkey today and is feeling sluggish.

sindresorhus added a commit that referenced this pull request Nov 27, 2015
Integrate time-require into the CLI.
@sindresorhus sindresorhus merged commit b370458 into avajs:master Nov 27, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants