Message ID | 817817a1-2f2d-1ee1-0898-f86801586d65@yandex.ru (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | generic/478: why eval is so fast? | expand |
On Fri, Jun 30, 2023 at 02:50:47PM +0500, stsp wrote: > Hi guys. > > While playing with the test 478, I've > noticed something very strange. Namely, > this small patch, that only adds eval to > the script: > > diff --git a/tests/generic/478 b/tests/generic/478 > index 480762d2..45b801d0 100755 > --- a/tests/generic/478 > +++ b/tests/generic/478 > @@ -106,24 +106,24 @@ do_test() > # print options and getlk output for debug > echo $* >> $seqres.full 2>&1 > # -s : do setlk > - $here/src/t_ofd_locks $soptions $TEST_DIR/testfile & > + eval $here/src/t_ofd_locks $soptions $TEST_DIR/testfile & > # -g : do getlk > - $here/src/t_ofd_locks $goptions $TEST_DIR/testfile | \ > + eval $here/src/t_ofd_locks $goptions $TEST_DIR/testfile | \ > tee -a $seqres.full > wait $! I didn't find out why it run faster, but I'm sure we can't merge a patch which only say "it's faster, but don't know why". Especially that "faster" is really unnormal. One thing I'm confused is about that background process, if you add eval to that line, can you sure the later "$!" (in "wait $!") is the t_ofd_locks process number? The second eval with "| tee -a $seqres.full" confused me too, I'm not so familar with eval, can't what kind of problem it brings in, but I doubt something wrong at there. Anyway, I can't merge a patch like this. But I don't mind if anyone can answer this question :) Thanks, Zorro > > # add -F to clone with CLONE_FILES > soptions="$1 -F" > # with -F, new locks are always file to place > - $here/src/t_ofd_locks $soptions $TEST_DIR/testfile & > - $here/src/t_ofd_locks $goptions $TEST_DIR/testfile | \ > + eval $here/src/t_ofd_locks $soptions $TEST_DIR/testfile & > + eval $here/src/t_ofd_locks $goptions $TEST_DIR/testfile | \ > tee -a $seqres.full > wait $! > > # add -d to dup and close > soptions="$1 -d" > - $here/src/t_ofd_locks $soptions $TEST_DIR/testfile & > - $here/src/t_ofd_locks $goptions $TEST_DIR/testfile | \ > + eval $here/src/t_ofd_locks $soptions $TEST_DIR/testfile & > + eval $here/src/t_ofd_locks $goptions $TEST_DIR/testfile | \ > tee -a $seqres.full > wait $! > } > > > speeds up the test by 50-60%! > This is quite unbelievable, and probably > means such change breaks something. > But I have no idea what and how. > Can anyone come up with a good guess? >
03.07.2023 20:05, Zorro Lang пишет: > I didn't find out why it run faster, but I'm sure we can't merge a > patch which only say "it's faster, but don't know why". Especially > that "faster" is really unnormal. > > One thing I'm confused is about that background process, if you add eval to > that line, can you sure the later "$!" (in "wait $!") is the t_ofd_locks > process number? > > The second eval with "| tee -a $seqres.full" confused me too, I'm not > so familar with eval, can't what kind of problem it brings in, but I > doubt something wrong at there. > > Anyway, I can't merge a patch like this. But I don't mind if anyone > can answer this question :) Hi, no, this wasn't a merge request, just a question. But the problem is that I need to merge another (not yet posted) patch that needs 1 of these evals to properly parse spaces in an arguments. The eval I need, is only in a line with "| tee" stuff, not the one for the process that would be later wait'ed for. Still, seeing such a huge speed-up, I was afraid to post the patch itself, and decided to ask first. Anyway, the patch in question waits for an approval of the semaphore fixing patch. It adds a new test, but the synchronization needs to work properly first.
diff --git a/tests/generic/478 b/tests/generic/478 index 480762d2..45b801d0 100755 --- a/tests/generic/478 +++ b/tests/generic/478 @@ -106,24 +106,24 @@ do_test() # print options and getlk output for debug echo $* >> $seqres.full 2>&1 # -s : do setlk - $here/src/t_ofd_locks $soptions $TEST_DIR/testfile & + eval $here/src/t_ofd_locks $soptions $TEST_DIR/testfile & # -g : do getlk - $here/src/t_ofd_locks $goptions $TEST_DIR/testfile | \ + eval $here/src/t_ofd_locks $goptions $TEST_DIR/testfile | \ tee -a $seqres.full wait $! # add -F to clone with CLONE_FILES soptions="$1 -F" # with -F, new locks are always file to place - $here/src/t_ofd_locks $soptions $TEST_DIR/testfile & - $here/src/t_ofd_locks $goptions $TEST_DIR/testfile | \ + eval $here/src/t_ofd_locks $soptions $TEST_DIR/testfile & + eval $here/src/t_ofd_locks $goptions $TEST_DIR/testfile | \ tee -a $seqres.full wait $! # add -d to dup and close soptions="$1 -d" - $here/src/t_ofd_locks $soptions $TEST_DIR/testfile & - $here/src/t_ofd_locks $goptions $TEST_DIR/testfile | \ + eval $here/src/t_ofd_locks $soptions $TEST_DIR/testfile & + eval $here/src/t_ofd_locks $goptions $TEST_DIR/testfile | \ tee -a $seqres.full wait $! }