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.

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

Jump to bottom

Investigate performance of 'compare' command for O(100K) utterances #138

Closed
rozele opened this issue Aug 23, 2019 · 1 comment · Fixed by #156
Closed

Investigate performance of 'compare' command for O(100K) utterances #138

rozele opened this issue Aug 23, 2019 · 1 comment · Fixed by #156

Comments

@rozele
Copy link
Contributor

rozele commented Aug 23, 2019

~100K utterances seems to be taking ~30 seconds for dotnet nlu compare -e tests100k.json -a tests100k.json -m. We should analyze the perf and see if there are any obvious ways to speedup the comparison.
rozele added a commit to rozele/NLU.DevOps that referenced this issue Sep 5, 2019
@rozele
Runs NUnit tests for compare command in parallel.
ff74292 
The bottleneck for the `compare` command is the exceptions thrown by NUnit for `Assert.Pass` and `Assert.Fail`. Running the tests in parallel will leverage more threads to run the tests. Another thing we can look into if we want to improve this further is to remove the calls to `Assert.Pass` and `Assert.Fail` and instead use expected outputs for unit tests, to remove the need for exception handling. For now, this parallel approach dramatically increases throughput for compare tests.

Fixes microsoft#138
@rozele
Copy link
Contributor Author

rozele commented Sep 5, 2019

The main bottleneck for compare tests is the exceptions that are thrown for Assert.Pass and Assert.Fail. As far as I can tell, these methods are the only way to specify a pass/fail reason for an NUnit test. A low-hanging way to improve performance is to make the tests run in parallel, which will come in a PR shortly. We can eventually also look into setting our "reason" as an additional metadata attribute on the test results (e.g., a category or something) and use something like expected results to pass/fail the NLU tests.

@rozele rozele mentioned this issue Sep 5, 2019
Merged
rozele added a commit that referenced this issue Sep 5, 2019
@rozele
Runs NUnit tests for compare command in parallel.
e0f5cfb 
The bottleneck for the `compare` command is the exceptions thrown by NUnit for `Assert.Pass` and `Assert.Fail`. Running the tests in parallel will leverage more threads to run the tests. Another thing we can look into if we want to improve this further is to remove the calls to `Assert.Pass` and `Assert.Fail` and instead use expected outputs for unit tests, to remove the need for exception handling. For now, this parallel approach dramatically increases throughput for compare tests.

Fixes #138
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Runs NUnit tests for compare command in parallel. rozele/NLU.DevOps
1 participant
@rozele