Non-optimal lseek usage & caching lseek

[David Collier-Brown]

  Ron Alexander wondered if we could eliminate some of
the redundant lseek system calls...   well, one way is
to cache the common case:

-- --- system.c.old        Thu Oct 12 10:33:03 2000
+++ system.c    Thu Oct 12 10:31:52 2000
@@ -144,6 +144,14 @@
 
 SMB_OFF_T sys_lseek(int fd, SMB_OFF_T offset, int whence)
 {
+  static int last_fd = 0;
+  static SMB_OFF_T last_offset = 0;
+
+  if (fd == last_fd && whence == SEEK_SET && offset == last_offset )
{
+       return last_offset;
+  }
+  last_fd = fd;
+  last_offset = offset;
 #if defined(HAVE_EXPLICIT_LARGEFILE_SUPPORT) && defined(HAVE_OFF64_T)
&& defined(HAVE_LSEEK64)
   return lseek64(fd, offset, whence);
 #else

  Historically, single-element caches catch something like 80% of the
calls to anything that's re-used.


  There are 34-odd calls to sys_lseek(..., SEEK_SET) out of a total
of 46 sys_lseek calls in 2.2, so this will probably help (;-))

  Looking at the SEEK_CUR usage, I see a 
lot of vfs_ops.lseek(fsp,fsp->fd,0,SEEK_CUR) calls, which
are no-ops!  In fact, only 1 of the SEEK_CURs is not a no-op.
We should eliminate the other 4 entirely.

  The SEEK_ENDs all look sane.

  Some of the SEEK_SETs looks suspicious: one pattern is
./lib/util.c:  if(sys_lseek(fd, currpos, SEEK_SET) != currpos)
./smbd/predict.c:  if (sys_lseek(rp_fd,rp_offset,SEEK_SET) !=
rp_offset) {
./passdb/passdb.c:  if(sys_lseek(fd, (SMB_OFF_T)0, SEEK_SET) != 0) {
./passdb/smbpass.c:  if (sys_lseek(fd, pwd_seekpos - 1, SEEK_SET) !=
pwd_seekpos - 1
./passdb/smbpass.c:  if (sys_lseek(fd, pwd_seekpos, SEEK_SET) !=
pwd_seekpos) {
./locking/locking_slow.c:  if(sys_lseek(fd, (SMB_OFF_T)0, SEEK_SET) !=
0)
./locking/locking_slow.c:    if(sys_lseek(fd, (SMB_OFF_T)0, SEEK_SET)
!= 0)
./locking/locking_slow.c:  if(sys_lseek(fd, (SMB_OFF_T)0, SEEK_SET) !=
0)
./locking/locking_slow.c:    if(sys_lseek(fd, (SMB_OFF_T)0, SEEK_SET)
!= 0)
./locking/locking_slow.c:  if(sys_lseek(fd, (SMB_OFF_T)0, SEEK_SET) !=
0)
./locking/locking_slow.c:  if(sys_lseek(fd, (SMB_OFF_T)0, SEEK_SET) !=
0)
./tdb/tdb.c:            if (lseek(tdb->fd, offset, SEEK_SET) != offset
||
./tdb/tdb.c:            if (lseek(tdb->fd, offset, SEEK_SET) != offset
||

  It appears that one of the team members checks seeks for "silently
failing", while others don't.  I suspect some vendor used to 
return a bogus offset, or the lp_postscript add-three-bytes code
failed during development: in normal cases lseek return -1 when it
can't seek...
  Can anyone say if we shouldn't be testing for -1?

  Write and tell me if I should submit a patch.

  I'll look at stat next, as that's the other performance-eater
mentioned here...

--dave

  



David Collier-Brown,  | Always do right. This will gratify some people
185 Ellerslie Ave.,   | and astonish the rest.        -- Mark Twain
Willowdale, Ontario   | //www.oreilly.com/catalog/samba/author.html
Work: (905) 415-2849 Home: (416) 223-8968 Email: davecb at canada.sun.com