public inbox for dash@vger.kernel.org
 help / color / mirror / Atom feed
From: Jan Pechanec <Jan.Pechanec@oracle.com>
To: dash@vger.kernel.org
Subject: dash performance regression with [ in latest github code
Date: Tue, 25 Feb 2025 16:04:40 +0100	[thread overview]
Message-ID: <Z73cCB4Xm89Y5AuT@len49> (raw)

Hi,

thank you for working on dash.  I was testing it recently and it worked
really well.

However, I noticed the dash code from github does filename pattern
matching even for code like "[ x = x ] && echo ok".  I believe the
unquoted space after '[' should not trigger pattern matching but rather
only to invoke the test/[ utility, as before.  It seems it works fine
though and only doing some extra unneeded work which may not be
immediatelly noticeable.

dash installed on my Oracle Linux 9:

janp:len49:~/_INST/dash$ strings /usr/bin/dash | grep dash
dash-0.5.11.5-4.el9.x86_64.debug
janp:len49:~/_INST/dash$ time dash -c 'i=0; while :; do : $((i=i+1)); [ $i -eq 500000 ] && break; done'

real    0m0.752s
user    0m0.748s
sys     0m0.002s

dash from github (commit b3e38adf6718801e7f06267b438c45caec9523bb) take
way more time to do the same thing:

janp:len49:~/_INST/dash$ time ./src/dash -c 'i=0; while :; do : $((i=i+1)); [ $i -eq 500000 ] && break; done'

real    0m4.202s
user    0m1.361s
sys     0m2.804s

For the latter, strace shows open, fstat, getdents*, and close system
calls for each iteration and it depends on number of files in the
current directory.  With more files, it takes more time:

janp:len49:/etc$ time ~/_INST/dash/src/dash -c 'i=0; while :; do : $((i=i+1)); [ $i -eq 500000 ] && break; done'
real    0m15.591s
user    0m5.704s
sys     0m9.828s

If I change [ to test, the dash github version behaves as before, and
possibly even faster:

janp:len49:~/_INST/dash$ time ~/_INST/dash/src/dash -c 'i=0; while :; do : $((i=i+1)); test $i -eq 500000 && break; done'

real    0m0.662s
user    0m0.659s
sys     0m0.002s

Even bash would be faster than the current github version of dash:

janp:len49:~/_INST/dash$ time bash -c 'i=0; while :; do : $((i=i+1)); [ $i -eq 500000 ] && break; done'
real    0m1.943s
user    0m1.939s
sys     0m0.002s

Unfortunately, I do not have time to work on a patch.

Best regards,
Jan

-- 
Jan Pechanec <jan.pechanec@oracle.com>

             reply	other threads:[~2025-02-25 15:04 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-25 15:04 Jan Pechanec [this message]
2025-02-25 15:13 ` dash performance regression with [ in latest github code Jan Pechanec
2025-03-09  9:42 ` [PATCH] expand: Add bypass for literal "]" in expandmeta Herbert Xu
2025-03-10  8:35   ` [External] : " Jan Pechanec

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z73cCB4Xm89Y5AuT@len49 \
    --to=jan.pechanec@oracle.com \
    --cc=dash@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox