Bug 26834

Summary:	Speed up get_overall() and get_bad()
Product:	libatasmart	Reporter:	Martin Pitt <martin.pitt>
Component:	library	Assignee:	Lennart Poettering <lennart>
Status:	RESOLVED FIXED	QA Contact:	Lennart Poettering <lennart>
Severity:	normal
Priority:	medium	CC:	zeuthen
Version:	unspecified
Hardware:	Other
OS:	All
Whiteboard:
i915 platform:		i915 features:
Attachments:	speed up patch

Description Martin Pitt 2010-03-02 01:06:59 UTC

Recently, udisks started to take a large amount of CPU time on startup (see the
"udisks-daemon" process line on [1]; the blue areas are CPU usage). Before
that, udisks had a negligible CPU usage (e. g. [2]).

This turned out to be due to a bug fix in libatasmart [3] which previously
broke SMART handling completely. So it's good to have that back. :-)

To examine that further, I added tracing to udisks (see [4]) which resulted in

  http://people.canonical.com/~pitti/bootcharts/udisks-20100225.png

This shows that sk_disk_smart_get_overall() takes about 0.5 s of CPU time on the Dell Mini 10 (with a slow Atom CPU), and still 0.3 s on my laptop (Core 2 Duo, where above chart was made).

For each attribute that get_overall() queries, it calls sk_disk_smart_parse_attributes() with a callback, which iterates over all
attributes, and the callback just picks out the one it's interested in. Thus
each single attribute query takes some .1 s.

I am working on a patch to reduce this to just one iteration over the attributes, and caching them in the SkDisk structure.

[1] http://people.canonical.com/~scott/daily-bootcharts/20100224-max.png
[2] http://people.canonical.com/~scott/daily-bootcharts/20100219.1-max.png
[3]
http://git.0pointer.de/?p=libatasmart.git;a=commitdiff;h=51502143eeb0a5553ab5977d07bf707dac47200c
[4] https://bugs.freedesktop.org/attachment.cgi?id=33560

Comment 1 Martin Pitt 2010-03-02 01:31:17 UTC

Created attachment 33683 [details] [review]
speed up patch

For comparing I added this to the beginning of get_overall():

        access("MARK: smart_get_overall(): start", F_OK);

and a corresponding "end" line to the end of the function, and called

   strace -e access -tttf -o /tmp/trace ./skdump  --load=blob-examples/FUJITSU_MHY2120BH--0084000D >/dev/null

This shows that with current git trunk, the function needs 0.305 s on my laptop.

With the caching patch, the time reduces to 0.0005 s. I can't really explain why it's that fast, since it only reduces the four sk_disk_smart_parse_attributes() calls to one, so it should have been 0.08 s. I guess there is some additional effect with CPU caches somewhere.

I diffed the output of

  for i in blob-examples/*; do echo "-- $i"; ./skdump --load=$i; done > /tmp/atasmart-test.out

without and with the patch, and there is no difference.

Comment 2 Lennart Poettering 2011-10-11 14:40:04 UTC

Hmm, so finally find the time to review this. Looks good. Applied. Thanks!

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.