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

[Tester] Faster tester and iut #223

Merged
merged 6 commits into from Oct 11, 2018
Merged

[Tester] Faster tester and iut #223

merged 6 commits into from Oct 11, 2018

Conversation

@shibatch
Copy link
Owner

@shibatch shibatch commented Aug 29, 2018

With this patch, testing time is reduced to 50%.

@shibatch shibatch requested a review from fpetrogalli Aug 29, 2018
Copy link
Collaborator

@fpetrogalli fpetrogalli left a comment

It would be great if you could revert the whitespace changes.

This is great stuff, we needed to make testing faster! Thank you for working on this.

@@ -4311,7 +4316,7 @@ void do_test() {

//

mpfr_set_default_prec(1280);
mpfr_set_default_prec(320);

This comment has been minimized.

@fpetrogalli

fpetrogalli Aug 31, 2018
Collaborator

Did you reduce the precision of MPFR because it gives speedup? Why did you choose the original values? Are these new values enough to make sure we don't miss errors?

This comment has been minimized.

@shibatch

shibatch Aug 31, 2018
Author Owner

1280 is for double precision.
320 is more than 128(exponent of single precision) + 24(mantissa length). Actually, 160 should be enough.

@@ -124,7 +124,11 @@ int readln(int fd, char *buf, int cnt) {
}

int startsWith(char *str, char *prefix) {
return strncmp(str, prefix, strlen(prefix)) == 0;
while(*str != '\0' && *prefix != '\0') {

This comment has been minimized.

@fpetrogalli

fpetrogalli Aug 31, 2018
Collaborator

Are you sure this code is faster then the library call? I believe that strncmp have highly specialized implementations in glibc...

This comment has been minimized.

@shibatch

shibatch Aug 31, 2018
Author Owner

The original code is strncmp + strlen.
This code is faster if defined as a static function.
I will move this to testerutil.h.

@@ -47,6 +47,7 @@ void stop(char *mes) {

int ptoc[2], ctop[2];
int pid;
FILE *fpctop;

This comment has been minimized.

@fpetrogalli

fpetrogalli Aug 31, 2018
Collaborator

Does it have to be global? Can't it be private inside of main?

This comment has been minimized.

@shibatch

shibatch Aug 31, 2018
Author Owner

fpctop is reference by many functions.

@@ -2,6 +2,7 @@
set -ev
cd /build/build-cross
make -j 2 all
export OMP_WAIT_POLICY=passive

This comment has been minimized.

@fpetrogalli

fpetrogalli Aug 31, 2018
Collaborator

What is this for?

This comment has been minimized.

@shibatch

shibatch Aug 31, 2018
Author Owner

This is for changing OpenMP setting.
With the default setting, it does not run fast if there are many OpenMP processes running simultaneously.

@fpetrogalli
Copy link
Collaborator

@fpetrogalli fpetrogalli commented Aug 31, 2018

@shibatch , where do you see the 50% speedup you claim in the PR?
When I look at https://travis-ci.org/shibatch/sleef/pull_requests, the run times seem comparable (~2h).

@shibatch
Copy link
Owner Author

@shibatch shibatch commented Aug 31, 2018

Running time at travis includes time for comiplation.
Compare, for example https://travis-ci.org/shibatch/sleef/jobs/422911913 and https://travis-ci.org/shibatch/sleef/jobs/418961450.
Testing time for iut is reduced from 165.04 sec to 89.70 sec.

@shibatch shibatch merged commit ffd58fe into master Oct 11, 2018
6 checks passed
6 checks passed
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@shibatch
continuous-integration/jenkins/branch This commit looks good
Details
@shibatch
continuous-integration/jenkins/pr-merge This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@shibatch shibatch deleted the Faster_tester2 branch Oct 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants