Skip to content

Added benchmarks.#90

Merged
gijsk merged 2 commits into
masterfrom
benchmarks
Apr 1, 2015
Merged

Added benchmarks.#90
gijsk merged 2 commits into
masterfrom
benchmarks

Conversation

@n1k0
Copy link
Copy Markdown
Contributor

@n1k0 n1k0 commented Apr 1, 2015

@gijsk
Copy link
Copy Markdown
Contributor

gijsk commented Apr 1, 2015

Can we get an average of a fixed set of those ops/s so it's easier to compare the output ?

@gijsk
Copy link
Copy Markdown
Contributor

gijsk commented Apr 1, 2015

(the total time for all benchmarks is less useful because new commits tend to add a new testcase)

@n1k0
Copy link
Copy Markdown
Contributor Author

n1k0 commented Apr 1, 2015

Can we get an average of a fixed set of those ops/s so it's easier to compare the output ?

I couldn't find an option for this, except exporting the result as CSV (brrr). We could create a separate suite with a fixed subset of these test pages, and mark it as a constant reference point, though.

@gijsk
Copy link
Copy Markdown
Contributor

gijsk commented Apr 1, 2015

Can we get an average of a fixed set of those ops/s so it's easier to compare the output ?

I couldn't find an option for this, except exporting the result as CSV (brrr). We could create a separate suite with a fixed subset of these test pages, and mark it as a constant reference point, though.

That sounds like a good idea, yeah.

@n1k0
Copy link
Copy Markdown
Contributor Author

n1k0 commented Apr 1, 2015

screen shot 2015-04-01 at 22 27 37

@n1k0
Copy link
Copy Markdown
Contributor Author

n1k0 commented Apr 1, 2015

@gijsk now we have these two distinct commands, npm run perf and npm run perf-reference, but we need to select the right subset of reference test pages. Could you please take over this, then land the patch when you're happy? It's a little late here :)

Comment thread index.js Outdated
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah just spotted: all this should be removed from this file, as we're not in a test context.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Done.

gijsk added a commit that referenced this pull request Apr 1, 2015
Added benchmarks to be able to keep track of the speed of the parser and readability.
@gijsk gijsk merged commit e51457d into master Apr 1, 2015
@gijsk gijsk deleted the benchmarks branch April 1, 2015 22:05
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