Project

General

Profile

Actions

Bug #13397

closed

ls calls time() many more times than necessary

Added by Peter Tribble 7 months ago. Updated about 2 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
cmd - userland programs
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

Investigating why my machine was reporting such a high number of sycalls, I stumbled across this in ls.c

rep->lat.tv_sec = time(NULL);
rep->lct.tv_sec = time(NULL);
rep->lmt.tv_sec = time(NULL);

For every file, ls calls time() 3 times, and only does so to ensure the values exist - the actual time here is arbitrary, and would normally be replaced by the times from the stat structure gotten from the file.

Removing those time() calls would make ls quicker, and reduce system load.


Files

13397.sh (2.35 KB) 13397.sh Peter Tribble, 2021-06-08 08:20 PM
Actions #1

Updated by Electric Monk 7 months ago

  • Gerrit CR set to 1121
Actions #2

Updated by Peter Tribble about 2 months ago

Tested with attached script 13397.sh, which verifies 3 things:
  • that the output is unchanged for some sample cases
  • that the new version is indeed quicker
  • that the expected number of syscalls has been reduced from 3 per file to just 3 in total
Actions #3

Updated by Peter Tribble about 2 months ago

Tested with attached script, output below, which quickly verifies that ls gives the same output as before (particularly the times).

Also a performance test, by running ls on /etc/motd and /etc 10,000 times. There's a clear performance improvement.

With about 157 files in /etc , this means that ls in the 2nd case looks at almost 1.6 million files, which means we save almost 5 million calls to time(). The observed change in the number of syscalls (from the cpu kstats) matches the expected delta with a little bit of noise.

Check -l /usr/bin
Check -lt /usr/bin
Check -ltr /usr/bin
Check -tr /usr/bin
Check -r /usr/bin
Check -t /usr/bin
Check -cl /usr/bin
Check -clt /usr/bin
Check -cltr /usr/bin
Check -ctr /usr/bin
Check -cr /usr/bin
Check -ct /usr/bin
Check -e /usr/bin
Check -et /usr/bin
Check -etr /usr/bin
Check -E /usr/bin
Check -Et /usr/bin
Check -Etr /usr/bin
Check -lu /usr/bin
Check -lur /usr/bin
Check -ur /usr/bin
Check -u /usr/bin
Check -l --time-style iso /usr/bin
Check -l --time-style long-iso /usr/bin
Check -l /system
Check -l /system/contract
Check -l /system/object
Check -l /etc
Check -l /etc/mnttab
Check -l /var/run

New time

real        0.020948749
user        0.009106877
sys         0.011570074
Old time

real        0.035248249
user        0.020944299
sys         0.013896450

Expect about 30000 difference
New syscalls 11603
Old syscalls 41700
Observed 30097

New time

real        0.989328766
user        0.715678917
sys         0.273229808
Old time

real        3.194135712
user        2.526556289
sys         0.667010077

Expect about 4680000 difference
New syscalls 77321
Old syscalls 4885264
Observed 4807943 difference

Actions #4

Updated by Electric Monk about 2 months ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit 6027b8601527b83fd174dc857073dfd37e9a3de8

commit  6027b8601527b83fd174dc857073dfd37e9a3de8
Author: Peter Tribble <peter.tribble@gmail.com>
Date:   2021-06-09T07:16:12.000Z

    13397 ls calls time() many more times than necessary
    Reviewed by: Richard Lowe <richlowe@richlowe.net>
    Reviewed by: Andy Fiddaman <andy@omnios.org>
    Reviewed by: Robert Mustacchi <rm@fingolfin.org>
    Approved by: Dan McDonald <danmcd@joyent.com>

Actions

Also available in: Atom PDF