Debian Bug report logs - #694668
A patch improve the performance of os-prober for systems with many partitions

Reply or subscribe to this bug.

Toggle useless messages

Message #5 received at submit@bugs.debian.org (full text, mbox, reply):

From: Hedayat Vatankhah <hedayatv@gmail.com>

To: submit@bugs.debian.org

Subject: A patch improve the performance of os-prober for systems with many partitions

Date: Thu, 29 Nov 2012 00:46:30 +0330

[Message part 1 (text/plain, inline)]

Package: os-prober
Version: 1.56
Tags: patch


This bug is reported by one of our users, and also the patch is provided 
by him. And this is his comments on the provided patch:

Here's a patch.  The elapsed wall-clock times in seconds are:
                      os-prober   linux-boot-prober
   os-prober-1.56        12.9           3.6
   patched                5.9           0.4

where os-prober analyzes all 43 partitions, but linux-boot-prober only 1.
So that is a factor of 9X for linux-boot-prober, but only 2.2X for 
os-prober.
During os-prober, "vmstat 2" shows CPU time about:
   10%  user
    9%  system
   20%  idle
   60%  wait

and indeed the hardware LED for "disk active" is ON almost all the time.
I do not understand why the idle and wait times are not close to zero.

The internal documentation is lacking.  I had to deduce the strategies.

Depending on usage, linux-boot-prober could be added to the os-prober 
pipeline.
(I changed linux-boot-prober to read stdin instead of using $1.)
The overlap should be enough to hide the entire time for linux-boot-prober.
(That would give 7X faster for the whole task as seen by user of anaconda.)
Also, there is no comment about the reason for using "blockdev --setro"
when "mount -ro" is used already.

My box has 3GHz dual CPU, fedora-18-Beta-RC1, and these disks:
   4  7200rpm SATA II drives; 2 GPT, 2 MSDOS
  32  linux root partitions
   5  data (non-root) partitions
   3  swap partitions (one per drive except Windows)
   2  not-formatted partitions
   1  Windows partition

Overview of significant changes in the patch:
1. Factor out thouands of invocations of 'loggger' into a single process
   which reads stdin.
2. Change strategy from serial tests to a shell pipeline where
   success goes "out the side" while failure "falls through" to next
   stage in pipeline.
3. Avoid 'grep' and 'sed' in favor of shell string processing, "set --"
   under appropriate "IFS=", and 'eval'.
4. Avoid logging the whole contents of grub.cfg.  Mine has 1955 lines,
   and anyway the patched logging would suffer rate-limiting by syslog.
5. Use the -a (AND) operator to '[' (test) instead of multiple tests
   with '&&'.

Because of its use by debian-installer, os-prober wants 'dash' as shell,
and not 'bash',  This is unfortunate because bash has '=~' for string 
maching,
"$(<" instead of "$(cat", and arrays.  Dash can use
"case "$param" in; pattern) ... ;; ... esac" for most simple uses of
" [[ "$param" =~ pattern ]] ", but somehow I find 'case' cumbersome.
So I use "_tmp="${param##pattern}"; if [ ${#_tmp} -ne ${#param} ]"
(etc.) to detect a match by difference in length.

[os-prober-1.56-ORIGIN.patch (text/x-patch, attachment)]

Message #10 received at 694668@bugs.debian.org (full text, mbox, reply):

From: Cyril Brulebois <kibi@debian.org>

To: Hedayat Vatankhah <hedayatv@gmail.com>, 694668@bugs.debian.org

Subject: Re: Bug#694668: A patch improve the performance of os-prober for systems with many partitions

Date: Tue, 25 Dec 2012 19:33:56 +0100

[Message part 1 (text/plain, inline)]

Hi,

Hedayat Vatankhah <hedayatv@gmail.com> (29/11/2012):
> This bug is reported by one of our users, and also the patch is
> provided by him. And this is his comments on the provided patch:
> […]

thanks for forwarding. Unfortunately it's too late in the release
cycle to consider this kind of large changes, so it's probably only
going to be considered during the 'jessie' release cycle.

Mraw,
KiBi.

[signature.asc (application/pgp-signature, inline)]

Message #15 received at 694668@bugs.debian.org (full text, mbox, reply):

From: Hedayat Vatankhah <hedayatv@gmail.com>

To: Cyril Brulebois <kibi@debian.org>

Cc: 694668@bugs.debian.org

Subject: Re: Bug#694668: A patch improve the performance of os-prober for systems with many partitions

Date: Wed, 23 Jan 2013 02:17:06 +0330

[Message part 1 (text/plain, inline)]

Yes, you are right specially since the patch does many things at once. I 
am going to break up the patch to smaller ones so that I can maintain 
them personally. I thought that you might also prefer this. Therefore, 
I'll post the smaller patches. But if you think that is not needed, let 
me know.
Anyway, attached you'll find a small patch which just replaces usage of 
'basename' with shell string processing (already used in some parts of 
the code, so should be supported in the target shell(s)).


/*Cyril Brulebois <kibi@debian.org>*/ wrote on Tue, 25 Dec 2012 19:33:56 
+0100:
> Hi,
>
> Hedayat Vatankhah <hedayatv@gmail.com> (29/11/2012):
>> This bug is reported by one of our users, and also the patch is
>> provided by him. And this is his comments on the provided patch:
>> […]
> thanks for forwarding. Unfortunately it's too late in the release
> cycle to consider this kind of large changes, so it's probably only
> going to be considered during the 'jessie' release cycle.
>
> Mraw,
> KiBi.

[Message part 2 (text/html, inline)]

[os-prober-remove-basename.patch (text/x-patch, attachment)]

Message #20 received at 694668@bugs.debian.org (full text, mbox, reply):

From: Hedayat Vatankhah <hedayatv@gmail.com>

To: 694668@bugs.debian.org

Subject: Re: Bug#694668: A patch improve the performance of os-prober for systems with many partitions

Date: Tue, 05 Feb 2013 22:56:24 +0330

[Message part 1 (text/plain, inline)]

A patch (part of the original) to pipe all logs to logger rather than 
running it repeatedly.

P.S. Isn't there any 'development' branch in os-prober which is not tied 
to Debian releases? That's weird.

[Message part 2 (text/html, inline)]

[os-prober-factor-out-logger.patch (text/x-patch, attachment)]

Send a report that this bug log contains spam.