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

fix(dstest): Cache the reading of TestCases, speeding up unit tests #141

Merged
merged 1 commit into from Aug 28, 2018

Conversation

dustmop
Copy link
Collaborator

@dustmop dustmop commented Aug 27, 2018

Our unit tests currently spend a lot of time re-reading and re-parsing test
files. Though these are quick individually, they add up to a significant
total of tests' runtimes, especially the craigslist/*.dataset.json files, which
are over 1MB and take about 1 second to parse. By caching these reads and
parses, and returning copies on future calls, we speed up tests by about 35%.

Some performance numbers follow.

Before this change:

Run 0
ok      github.com/qri-io/qri/api      25.587s
ok      github.com/qri-io/qri/cmd      84.258s
ok      github.com/qri-io/qri/config   48.832s
ok      github.com/qri-io/qri/lib      90.485s
ok      github.com/qri-io/qri/p2p      54.261s
Run 1
ok      github.com/qri-io/qri/api      30.159s
ok      github.com/qri-io/qri/cmd      79.327s
ok      github.com/qri-io/qri/config   40.614s
ok      github.com/qri-io/qri/lib      74.311s
ok      github.com/qri-io/qri/p2p      43.396s
Run 2
ok      github.com/qri-io/qri/api      23.189s
ok      github.com/qri-io/qri/cmd      79.591s
ok      github.com/qri-io/qri/config   44.684s
ok      github.com/qri-io/qri/lib      75.476s
ok      github.com/qri-io/qri/p2p      41.786s

After this change:

Run 0
ok      github.com/qri-io/qri/api      18.428s
ok      github.com/qri-io/qri/cmd      59.639s
ok      github.com/qri-io/qri/config   46.602s
ok      github.com/qri-io/qri/lib      52.682s
ok      github.com/qri-io/qri/p2p      40.562s
Run 1
ok      github.com/qri-io/qri/api      21.280s
ok      github.com/qri-io/qri/cmd      55.533s
ok      github.com/qri-io/qri/config   51.034s
ok      github.com/qri-io/qri/lib      44.018s
ok      github.com/qri-io/qri/p2p      49.323s
Run 2
ok      github.com/qri-io/qri/api      15.110s
ok      github.com/qri-io/qri/cmd      57.857s
ok      github.com/qri-io/qri/config   43.257s
ok      github.com/qri-io/qri/lib      49.521s
ok      github.com/qri-io/qri/p2p      36.730s

@ghost ghost assigned dustmop Aug 27, 2018
@ghost ghost added the in progress label Aug 27, 2018
@dustmop dustmop requested a review from ramfox August 27, 2018 21:37
Our unit tests currently spend a lot of time re-reading and re-parsing test
files. Though these are quick individually, they add up to a significant
total of tests' runtimes, especially the craigslist/*.dataset.json files, which
are over 1MB and take about 1 second to parse. By caching these reads and
parses, and returning copies on future calls, we speed up tests by about 35%.

Some performance numbers follow.

Before this change:
Run 0
ok      github.com/qri-io/qri/api      25.587s
ok      github.com/qri-io/qri/cmd      84.258s
ok      github.com/qri-io/qri/config   48.832s
ok      github.com/qri-io/qri/lib      90.485s
ok      github.com/qri-io/qri/p2p      54.261s
Run 1
ok      github.com/qri-io/qri/api      30.159s
ok      github.com/qri-io/qri/cmd      79.327s
ok      github.com/qri-io/qri/config   40.614s
ok      github.com/qri-io/qri/lib      74.311s
ok      github.com/qri-io/qri/p2p      43.396s
Run 2
ok      github.com/qri-io/qri/api      23.189s
ok      github.com/qri-io/qri/cmd      79.591s
ok      github.com/qri-io/qri/config   44.684s
ok      github.com/qri-io/qri/lib      75.476s
ok      github.com/qri-io/qri/p2p      41.786s

After this change:
Run 0
ok      github.com/qri-io/qri/api      18.428s
ok      github.com/qri-io/qri/cmd      59.639s
ok      github.com/qri-io/qri/config   46.602s
ok      github.com/qri-io/qri/lib      52.682s
ok      github.com/qri-io/qri/p2p      40.562s
Run 1
ok      github.com/qri-io/qri/api      21.280s
ok      github.com/qri-io/qri/cmd      55.533s
ok      github.com/qri-io/qri/config   51.034s
ok      github.com/qri-io/qri/lib      44.018s
ok      github.com/qri-io/qri/p2p      49.323s
Run 2
ok      github.com/qri-io/qri/api      15.110s
ok      github.com/qri-io/qri/cmd      57.857s
ok      github.com/qri-io/qri/config   43.257s
ok      github.com/qri-io/qri/lib      49.521s
ok      github.com/qri-io/qri/p2p      36.730s
@ramfox
Copy link
Member

ramfox commented Aug 27, 2018

💰 💸 💵

@dustmop dustmop merged commit 2c68cce into master Aug 28, 2018
@ghost ghost removed the in progress label Aug 28, 2018
@dustmop dustmop deleted the cache-tc-reads branch August 28, 2018 00:54
dustmop added a commit to qri-io/qri that referenced this pull request Aug 28, 2018
Currently, the p2p tests (as well as some tests in lib) spend a lot of time
generating actual crypto keys due to calling DefaultP2P, which are then
replaced by test keys from the libp2p package. Avoiding this useless work
speeds up the p2p tests by about 5x.

Performance numbers from before this change, taken from qri-io/dataset#141:
Run 0
ok      github.com/qri-io/qri/api      18.428s
ok      github.com/qri-io/qri/cmd      59.639s
ok      github.com/qri-io/qri/config   46.602s
ok      github.com/qri-io/qri/lib      52.682s
ok      github.com/qri-io/qri/p2p      40.562s
Run 1
ok      github.com/qri-io/qri/api      21.280s
ok      github.com/qri-io/qri/cmd      55.533s
ok      github.com/qri-io/qri/config   51.034s
ok      github.com/qri-io/qri/lib      44.018s
ok      github.com/qri-io/qri/p2p      49.323s
Run 2
ok      github.com/qri-io/qri/api      15.110s
ok      github.com/qri-io/qri/cmd      57.857s
ok      github.com/qri-io/qri/config   43.257s
ok      github.com/qri-io/qri/lib      49.521s
ok      github.com/qri-io/qri/p2p      36.730s

After this change:
Run 0
ok      github.com/qri-io/qri/api      13.866s
ok      github.com/qri-io/qri/cmd      61.417s
ok      github.com/qri-io/qri/config   32.168s
ok      github.com/qri-io/qri/lib      32.685s
ok      github.com/qri-io/qri/p2p       8.008s
Run 1
ok      github.com/qri-io/qri/api      18.329s
ok      github.com/qri-io/qri/cmd      68.314s
ok      github.com/qri-io/qri/config   43.116s
ok      github.com/qri-io/qri/lib      32.584s
ok      github.com/qri-io/qri/p2p       6.926s
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.

None yet

2 participants