Re: "ls -l": Avoid unnecessary getxattr() overhead

[Bernhard Voelker]

On 02/17/2012 04:15 PM, Jim Meyering wrote:
> And here's the patch I've just squashed into the test c-set.
> 
> diff --git a/tests/ls/getxattr-speedup b/tests/ls/getxattr-speedup
> index eaa3342..a3d47c1 100755
> --- a/tests/ls/getxattr-speedup
> +++ b/tests/ls/getxattr-speedup
> @@ -28,16 +28,18 @@ fd=33
>  # determine the total number of calls.  FIXME: there must be a better way.
>  cat > k.c <<'EOF' || framework_failure_
>  #include <errno.h>
> -#include <unistd.h>
> -static void
> -write_1 (void)
> -{
> -  write (FD, "x\n", 2);
> -}
> +#include <stdio.h>
> +static unsigned long int n_calls;
>  ssize_t getxattr (const char *path, const char *name, void *value, size_t 
> size)
> -{ write_1 (); errno = ENOTSUP; return -1; }
> +{ ++n_calls; errno = ENOTSUP; return -1; }
>  ssize_t lgetxattr(const char *path, const char *name, void *value, size_t 
> size)
> -{ write_1 (); errno = ENOTSUP; return -1; }
> +{ ++n_calls; errno = ENOTSUP; return -1; }
> +__attribute__((destructor)) void p()
> +{
> +  FILE *fp = fdopen (FD, "a"); if (!fp) return;
> +  fprintf (fp, "%lu\n", n_calls);
> +  fclose (fp);
> +}
>  EOF
> 
>  # Then compile/link it:
> @@ -52,7 +54,7 @@ seq 100 | xargs touch || framework_failure_
>  eval "LD_PRELOAD=./k.so ls --color=always -l . $fd>x" || fail=1
> 
>  # Ensure that there were no more than 3 *getxattr calls.
> -n_calls=$(wc -l < x)
> +n_calls=$(cat x)
>  test "$n_calls" -le 3 || fail=1
> 
>  Exit $fail

Hmm, this doesn't work here - "x" is not created :-(

++ cat x
+ n_calls=
+ test '' -le 3
./ls/getxattr-speedup: line 58: test: : integer expression expected

gcc (SUSE Linux) 4.6.2
openSUSE 12.1 (x86_64)

Another suggestion: writing into "x" in write_1(). By this, you can
also get rid of fd=33 ... admittedly to the cost of maybe several
fopen calls. On my system, n_calls doesn't get more than 1.
We could yet s/seq 100/seq 20/.


diff --git a/tests/ls/getxattr-speedup b/tests/ls/getxattr-speedup
index ecdd126..60a72ed 100755
--- a/tests/ls/getxattr-speedup
+++ b/tests/ls/getxattr-speedup
@@ -21,18 +21,18 @@
 . "${srcdir=.}/init.sh"; path_prepend_ ../src
 print_ver_ ls

-fd=33
-
 # Replace each getxattr and lgetxattr call with a call to these stubs.
-# Each writes a single line to the specified FD so that we can later
-# determine the total number of calls.  FIXME: there must be a better way.
+# Each writes the number of calls into "x".
 cat > k.c <<'EOF' || framework_failure_
 #include <errno.h>
-#include <unistd.h>
+#include <stdio.h>
 static void
 write_1 (void)
 {
-  write (FD, "x\n", 2);
+  static unsigned long int n_calls;
+  ++n_calls;
+  FILE *fp = fopen ("x", "w"); if (!fp) return;
+  fprintf (fp, "%lu\n", n_calls);
 }
 ssize_t getxattr (const char *path, const char *name, void *value, size_t size)
 { write_1 (); errno = ENOTSUP; return -1; }
@@ -41,7 +41,7 @@ ssize_t lgetxattr(const char *path, const char *name, void 
*value, size_t size)
 EOF

 # Then compile/link it:
-$CC -DFD=$fd -fPIC -O2 -c k.c \
+$CC -fPIC -O2 -c k.c \
   || framework_failure_ 'failed to compile with -fPIC'
 ld -G k.o -o k.so || framework_failure_ 'failed to invoke ld -G ...'

@@ -49,10 +49,10 @@ ld -G k.o -o k.so || framework_failure_ 'failed to invoke 
ld -G ...'
 seq 100 | xargs touch || framework_failure_

 # Finally, run the test, redirecting
-eval "LD_PRELOAD=$PWD/k.so ls --color=always -l . $fd>x" || fail=1
+eval "LD_PRELOAD=./k.so ls --color=always -l ." || fail=1

 # Ensure that there were no more than 3 *getxattr calls.
-n_calls=$(wc -l < x)
+n_calls=$(cat x)
 test "$n_calls" -le 3 || fail=1

 Exit $fail