1.80.0: parallel test suite fails #567
Comments
|
If you have 6 cores you may be able to get away with using -j2. BTW, I don't recognize this error message: |
|
closing this issue |
|
So .. there is no bug here? |
|
WAI. With j2 you can get a race condition between the different cores. Depends on how many cores you have. |
|
I have have 24 phicicel cores. |
|
You could extend diff --git a/prog/reg_wrapper.sh b/prog/reg_wrapper.sh
index 38502f5..3511c2b 100755
--- a/prog/reg_wrapper.sh
+++ b/prog/reg_wrapper.sh
@@ -45,4 +45,4 @@ case "${TEST_NAME}" in
fi
esac
-exec ${@%${TEST}} /bin/sh -c "cd \"${srcdir}\" && \"${PWD}/\"${TEST} generate && \"${PWD}/\"${TEST} compare"
+exec ${@%${TEST}} /bin/sh -c "export LEPT_TMPDIR=\"${PWD}/${TEST_NAME}.tmp\"; cd \"${srcdir}\" && \"${PWD}/\"${TEST} generate && \"${PWD}/\"${TEST} compare"
diff --git a/src/utils2.c b/src/utils2.c
index aabf288..2a5c213 100644
--- a/src/utils2.c
+++ b/src/utils2.c
@@ -3108,6 +3108,7 @@ genPathname
return (char *)ERROR_PTR("pathout not made", __func__, NULL);
}
+ char *lept_tmpdir = getenv("LEPT_TMPDIR");
#ifdef _WIN32
is_win32 = TRUE;
#endif /* _WIN32 */
@@ -3116,10 +3117,15 @@ size_t size;
* There is no path rewriting on unix, and on win32, we do not
* rewrite unless the specified directory is /tmp or
* a subdirectory of /tmp */
- if (!is_win32 || dirlen < 4 ||
+ if ((!is_win32 && lept_tmpdir == NULL) || dirlen < 4 ||
(dirlen == 4 && strncmp(cdir, "/tmp", 4) != 0) || /* not in "/tmp" */
(dirlen > 4 && strncmp(cdir, "/tmp/", 5) != 0)) { /* not in "/tmp/" */
stringCopy(pathout, cdir, dirlen);
+ } else if (lept_tmpdir != NULL) {
+ stringCopy(pathout, lept_tmpdir, strlen(lept_tmpdir));
+ /* Add the rest of cdir */
+ if (dirlen > 4)
+ stringCat(pathout, size, cdir + 4);
} else { /* Rewrite for win32 with "/tmp" specified for the directory. */
#ifdef _WIN32
l_int32 tmpdirlen;I also had to fix 3 additional issues:
diff --git a/prog/kernel_reg.c b/prog/kernel_reg.c
index 997d0d7..a288fe6 100644
--- a/prog/kernel_reg.c
+++ b/prog/kernel_reg.c
@@ -45,7 +45,7 @@ int main(int argc,
char **argv)
{
char *str;
-l_int32 i, j, same, ok, plottype;
+l_int32 i, j, same, ok;
l_float32 sum, avediff, rmsdiff;
L_KERNEL *kel1, *kel2, *kel3, *kel4, *kelx, *kely;
BOX *box;
@@ -222,8 +222,7 @@ L_REGPARAMS *rp;
pixWrite("/tmp/lept/regout/conv2.png", pixt2, IFF_PNG); /* ditto */
regTestCheckFile(rp, "/tmp/lept/regout/conv2.png"); /* 11 */
- plottype = (rp->display) ? GPLOT_PNG : 0;
- pixCompareGray(pixt, pixt2, L_COMPARE_ABS_DIFF, plottype, NULL,
+ pixCompareGray(pixt, pixt2, L_COMPARE_ABS_DIFF, GPLOT_PNG, NULL,
&avediff, &rmsdiff, NULL);
pixp = pixRead("/tmp/lept/comp/compare_gray0.png");
pixaAddPix(pixa, pixp, L_INSERT);
diff --git a/prog/numa3_reg.c b/prog/numa3_reg.c
index d15cff1..960d333 100644
--- a/prog/numa3_reg.c
+++ b/prog/numa3_reg.c
@@ -70,7 +70,7 @@ L_REGPARAMS *rp;
pixs = pixRead("test8.jpg");
nasy= pixGetGrayHistogramMasked(pixs, NULL, 0, 0, 1);
numaMakeRankFromHistogram(0.0, 1.0, nasy, 350, &nax, &nay);
- pix1 = gplotGeneralPix2(nax, nay, GPLOT_LINES, "/tmp/lept/numa1/rank1",
+ pix1 = gplotGeneralPix2(nax, nay, GPLOT_LINES, "/tmp/lept/numa3/rank1",
"test rank extractor", "pix val", "rank val");
numaDestroy(&nasy);
numaDestroy(&nax);
@@ -86,7 +86,7 @@ L_REGPARAMS *rp;
numaHistogramGetValFromRank(na, rank, &val);
numaAddNumber(nap, val);
}
- pix2 = gplotGeneralPix1(nap, GPLOT_LINES, "/tmp/lept/numa1/rank2",
+ pix2 = gplotGeneralPix1(nap, GPLOT_LINES, "/tmp/lept/numa3/rank2",
"rank value", NULL, NULL);
pixa = pixaCreate(2);
regTestWritePixAndCheck(rp, pix1, IFF_PNG); /* 0 */
@@ -107,19 +107,19 @@ L_REGPARAMS *rp;
* Numa-morphology *
* -------------------------------------------------------------------*/
na = numaRead("lyra.5.na");
- pix1 = gplotGeneralPix1(na, GPLOT_LINES, "/tmp/lept/numa1/lyra1",
+ pix1 = gplotGeneralPix1(na, GPLOT_LINES, "/tmp/lept/numa3/lyra1",
"Original", NULL, NULL);
na1 = numaErode(na, 21);
- pix2 = gplotGeneralPix1(na1, GPLOT_LINES, "/tmp/lept/numa1/lyra2",
+ pix2 = gplotGeneralPix1(na1, GPLOT_LINES, "/tmp/lept/numa3/lyra2",
"Erosion", NULL, NULL);
na2 = numaDilate(na, 21);
- pix3 = gplotGeneralPix1(na2, GPLOT_LINES, "/tmp/lept/numa1/lyra3",
+ pix3 = gplotGeneralPix1(na2, GPLOT_LINES, "/tmp/lept/numa3/lyra3",
"Dilation", NULL, NULL);
na3 = numaOpen(na, 21);
- pix4 = gplotGeneralPix1(na3, GPLOT_LINES, "/tmp/lept/numa1/lyra4",
+ pix4 = gplotGeneralPix1(na3, GPLOT_LINES, "/tmp/lept/numa3/lyra4",
"Opening", NULL, NULL);
na4 = numaClose(na, 21);
- pix5 = gplotGeneralPix1(na4, GPLOT_LINES, "/tmp/lept/numa1/lyra5",
+ pix5 = gplotGeneralPix1(na4, GPLOT_LINES, "/tmp/lept/numa3/lyra5",
"Closing", NULL, NULL);
pixa = pixaCreate(2);
pixaAddPix(pixa, pix1, L_INSERT);
@@ -157,7 +157,7 @@ L_REGPARAMS *rp;
na3 = numaTransform(na2, 0.0, 1.0 / maxval);
numaFindLocForThreshold(na3, 0, &thresh, NULL);
numaAddNumber(na4, thresh);
- snprintf(buf1, sizeof(buf1), "/tmp/lept/numa1/histoplot-%d", hw);
+ snprintf(buf1, sizeof(buf1), "/tmp/lept/numa3/histoplot-%d", hw);
snprintf(buf2, sizeof(buf2), "halfwidth = %d, skip = 20, thresh = %d",
hw, thresh);
pix1 = gplotGeneralPix1(na3, GPLOT_LINES, buf1, buf2, NULL, NULL);
@@ -167,12 +167,12 @@ L_REGPARAMS *rp;
numaDestroy(&na2);
numaDestroy(&na3);
}
- numaWrite("/tmp/lept/numa1/threshvals.na", na4);
- regTestCheckFile(rp, "/tmp/lept/numa1/threshvals.na"); /* 9 */
- L_INFO("writing /tmp/lept/numa1/histoplots.pdf\n", "numa1_reg");
+ numaWrite("/tmp/lept/numa3/threshvals.na", na4);
+ regTestCheckFile(rp, "/tmp/lept/numa3/threshvals.na"); /* 9 */
+ L_INFO("writing /tmp/lept/numa3/histoplots.pdf\n", "numa3_reg");
pixaConvertToPdf(pixa, 0, 1.0, L_FLATE_ENCODE, 0,
"Effect of smoothing on threshold value",
- "/tmp/lept/numa1/histoplots.pdf");
+ "/tmp/lept/numa3/histoplots.pdf");
numaDestroy(&na1);
numaDestroy(&na4);
pixaDestroy(&pixa);
diff --git a/src/sarray1.c b/src/sarray1.c
index c04079d..a9e876e 100644
--- a/src/sarray1.c
+++ b/src/sarray1.c
@@ -1877,12 +1877,8 @@ SARRAY *saout;
SARRAY *
getFilenamesInDirectory(const char *dirname)
{
-#if _POSIX_VERSION >= 200112 || defined(__GLIBC__)
-char *dir;
-#else
char dir[PATH_MAX + 1];
-#endif
-char *realdir, *stat_path, *ignore;
+char *realdir, *stat_path, *gendir;
size_t size;
SARRAY *safiles;
DIR *pdir;
@@ -1898,34 +1894,13 @@ struct stat st;
/* Who would have thought it was this fiddly to open a directory
and get the files inside? fstatat() works with relative
directory paths, and stat() requires using the absolute path.
- realpath works as follows for files and directories:
- * If the file or directory exists, realpath returns its path;
- else it returns NULL.
- * If the second arg to realpath is passed in, the canonical path
- is returned there. Use a buffer of sufficient size.
- We pass in a buffer for the second arg, and check that the
- canonical directory path was made. The existence of the
- directory is checked later, after its actual path is returned by
- genPathname().
- With GNU libc or Posix 2001, if the second arg is NULL, the path
- is malloc'd and returned if the file or directory exists.
*/
-#if _POSIX_VERSION >= 200112 || defined(__GLIBC__)
- dir = realpath(dirname, NULL);
- if (dir == NULL)
- return (SARRAY *)ERROR_PTR("dir not made", __func__, NULL);
-#else
- dir[0] = '\0'; /* init empty in case realpath() fails to write it */
- ignore = realpath(dirname, dir);
- if (dir[0] == '\0')
+ gendir = genPathname(dirname, NULL);
+ realdir = realpath(gendir, dir);
+ LEPT_FREE(gendir);
+ if (realdir == NULL)
return (SARRAY *)ERROR_PTR("dir not made", __func__, NULL);
-#endif
- realdir = genPathname(dir, NULL);
-#if _POSIX_VERSION >= 200112 || defined(__GLIBC__)
- LEPT_FREE(dir);
-#endif
if ((pdir = opendir(realdir)) == NULL) {
- LEPT_FREE(realdir);
return (SARRAY *)ERROR_PTR("pdir not opened", __func__, NULL);
}
safiles = sarrayCreate(0);
@@ -1953,7 +1928,6 @@ struct stat st;
sarrayAddString(safiles, pdirentry->d_name, L_COPY);
}
closedir(pdir);
- LEPT_FREE(realdir);
return safiles;
}
Happy to PR all/some of those changes if you're interested. |
|
Thank you for doing all that, Pierre! I've made the changes in 3 of your 4 suggestions. The getFilenamesInDirectory() simplification is nice. I decided to require the POSIX 2008 non-buggy version of realpath(), where the path is malloc'd. With luck there will not be complaints. A fix for the regression test The suggestion not implemented is the genPathname() rewrite for windows with a special environment variable. I'm not convinced there is a problem to be solved. BTW, for stringCopy() you must limit the copy using strlen(pathout), the buffer size. |
Great!
It would make running the testsuite much faster (~4x faster on my old desktop, YMMV). I also think it might fix the problem #561 is trying to address, which I'm pretty sure I hit too here, when adding leptonica support to Meson's WrapDB. I'm happy to contribute that support here BTW, if you want.
You mean to avoid an overflow if |
|
How can this affect the running time? stringCopy: just the usual thing of limiting the number of copied bytes to the size of the buffer (not the size of the source string) |
|
The function |
|
Here is the full list of temporary paths common to multiple tests:
Generated after running the tests (using #708) and the following command: (
for d in prog/*.tmp; do
t=${d##*/};
t=${t%.tmp};
find $d -mindepth 1 -printf '%P\n' | sed "s,^,$t /tmp/,";
done | sort -k2 -t' ';
echo
) | awk -f tmpfiles.awk
{
if (path != $2) {
if (count > 1)
print "`"path"`:", (count == 147 ? "all tests" : tests)
path = $2
count = 0
tests = ""
}
count += 1
tests = (count > 1 ? tests", " : "") $1
} |
As long as test suite is started with
-j1everything is OK.The text was updated successfully, but these errors were encountered: