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

crc: a proposal for implementation of crc test-case #67

Open
zhiyuan-zhu opened this issue Apr 23, 2019 · 9 comments
Open

crc: a proposal for implementation of crc test-case #67

zhiyuan-zhu opened this issue Apr 23, 2019 · 9 comments

Comments

@zhiyuan-zhu
Copy link
Contributor

@gbtucker
This patch used to stress test for crc algorithm, especially the
clmul (carry-less multiplication instruction) optimized version.

More detailed, see the attachment file:
design-doc.md.txt

@gbtucker
Copy link
Contributor

@zhiyuan-zhu, thanks for proposal to add test cases. I agree the crc update test and sizes could be improved. The update test could take random fragment sizes for example.

I'm not sure if reading data from a file will help since the program flow is completely feed-forward, the control flow of the program doesn't depend on the data. However we could model such as in igzip_rand_test.c where the data is either synthetically generated or taken from a file if provided.

Also I don't think adding the table-driven comparisons in the test will speed up the tests. The reference functions are much slower, will dominate the time of the tests, and must be performed anyway. Adding another comparison will just slow the tests down and is really unnecessary if we also run the noarch tests.

@zhiyuan-zhu
Copy link
Contributor Author

zhiyuan-zhu commented Apr 24, 2019

@gbtucker, thanks your suggestions, I will redesign this proposal. For "reading data from a file", I will refer to igzip_rand_test.c's design.

I would like to create a new directory named: crc_stability_test in the crc/ directory, let this new test case not affect the original test case and as an option for users and developers. So the default original test case will still guarantee the correctness of the basic functions, this stability test case will just compare table-driven version with multi-binary version (not basic bit-wise version).

@zhiyuan-zhu
Copy link
Contributor Author

@gbtucker, refine the proposal.

  1. Create a new directory "crc_stability" under the crc/ directory for this new stability test case.
  2. Using a longer buffer size.
  3. Compare table-driven version with multi-binary version crc algorithm, the existing test case as basic functionally test, and this test case will refer to the existing test case.
  4. Provides the ability to read data from files as an option for command line arguments

Would you like to accept this proposal or have any other suggestions? thanks!

@gbtucker
Copy link
Contributor

@zhiyuan-zhu I suggest we concentrate on test cases to improve coverage and not more comparisons. I think more cases and longer buffer sizes can still fit in the current "check" test. If test runtime is an issue then it can move to an extended test unit_tests += No need to make a new category and or directory. If we keep the test to compare just reference vs. multi-binary then we can get more comparisons by running on lots of arch, which we need to do anyway, and the noarch check, which we already do. I suggest we improve incrementally by adding one new testcase at a time to the existing test. New tests can be parameterized by size, dataset, etc. and run multiple times if need be.

@zhiyuan-zhu
Copy link
Contributor Author

@gbtucker Thanks a lot, I will refine this proposal soon :).

@zhiyuan-zhu
Copy link
Contributor Author

@gbtucker refine the proposal.

  1. Enhance the strength of existing test cases.
  2. Using a longer buffer size.
  3. Extended test used "unit_tests +=" for Compare table-driven version with multi-binary version crc algorithm.
  4. Provides the ability to read data from files as an option for command line arguments.
  5. Parameterized by size, dataset, run multiple times.

Do you have any other suggestions?

@gbtucker
Copy link
Contributor

gbtucker commented May 7, 2019

@zhiyuan-zhu, I like 1, 2, & 5. Let's do those first. I'm not sure 3 & 4 add much.

@zhiyuan-zhu
Copy link
Contributor Author

@gbtucker Thanks for your kindly reply, I will arrange a more detailed design document follow the blew items.

  1. Enhance the strength of existing test cases.
  2. Using a longer buffer size.
  3. Parameterized by size, dataset, run multiple times.
    @cyb70289

@zhiyuan-zhu
Copy link
Contributor Author

@gbtucker

I have upload two documents,

  1. detailed-design-doc.md.txt for crc test case improve, detailed design document.
  2. perf-test-improve-proposal.md.txt for crc perf test improve proposal.
    Would you please help to take a look at them, thanks.

detailed-design-doc.md.txt
perf-test-improve-proposal.md.txt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants