From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
To: Tomi Ollila <tomi.ollila@iki.fi>, Tomi Ollila <tomi.ollila@iki.fi>,
	notmuch@notmuchmail.org
Subject: Re: [PATCH 1/3] test: add functions to count how much times notmuch
	was called
In-Reply-To: <yf6mxbfxezr.fsf@taco2.nixu.fi>
References: <1322271878-32614-1-git-send-email-dmitry.kurochkin@gmail.com>
	<1322450895-32523-1-git-send-email-dmitry.kurochkin@gmail.com>
	<1322450895-32523-2-git-send-email-dmitry.kurochkin@gmail.com>
	<yf6aa7gj7w5.fsf@taco2.nixu.fi> <87hb1ovsz4.fsf@gmail.com>
	<yf6mxbfxezr.fsf@taco2.nixu.fi>
User-Agent: Notmuch/0.10+44~g067c44f (http://notmuchmail.org) Emacs/23.3.1
	(x86_64-pc-linux-gnu)
Date: Wed, 30 Nov 2011 01:03:27 +0400
Message-ID: <87aa7evdy8.fsf@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

Hi Tomi.

On Tue, 29 Nov 2011 14:58:00 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> Hi Dmitry.
> 
> On Tue, 29 Nov 2011 01:26:39 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > Hi Tomi.
> > 
> > On Mon, 28 Nov 2011 22:42:50 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> > > On Mon, 28 Nov 2011 07:28:13 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> [ ... ]
> > > > +# Creates a script that counts how much time it is executed and calls
> > > > +# notmuch.  $notmuch_counter_command is set to the path to the
> > > > +# generated script.  Use notmuch_counter_value() function to get the
> > > > +# current counter value.
> > > > +notmuch_counter_reset () {
> > > > +	notmuch_counter_command="$TMP_DIRECTORY/notmuch_counter"
> > > > +	if [ ! -x "$notmuch_counter_command" ]; then
> > > > +		notmuch_counter_state_path="$TMP_DIRECTORY/notmuch_counter.state"
> > > > +		cat >"$notmuch_counter_command" <<EOF || return
> > > > +#!/bin/sh
> > > > +
> > > > +count=\$(cat "$notmuch_counter_state_path")
> > > > +echo -n \$(expr \$count + 1) > "$notmuch_counter_state_path"
> > > > +
> > > > +exec notmuch "\$@"
> > > > +EOF
> > > > +		chmod +x "$notmuch_counter_command" || return
> > > > +	fi
> > > > +
> > > > +	echo -n 0 > "$notmuch_counter_state_path"
> > > > +}
> > > > +
> > > > +# Returns the current notmuch counter value.
> > > > +notmuch_counter_value () {
> > > > +	if [ -r "$notmuch_counter_state_path" ]; then
> > > > +		count=$(cat "$notmuch_counter_state_path")
> > > > +	else
> > > > +		count=0
> > > > +	fi
> > > > +	echo -n $count
> > > > +}
> > > > +
> > > 
> > > Good work! It would be nice if the state file contained newline after
> > > count number.
> > 
> > I wonder why it is actually nice :)  I do not have strong preference
> > here.  So a newline is added in v3.  Also a newline is added to
> > notmuch_counter_value() output for consistency.
> 
> It is nice when I enter cat /path/to/notmuch_counter from command
> line and shell prompt is not appended at the end of the file contents
> (but on next line :)
> 
> > 
> > > Also some optimizations could be done:
> > > 
> > 
> > (Would be nice if you send a diff, or a human-friendly description of
> > the changes.)
> 
> Ok, I'll try to do this according to your wishes next time.
> 
> > > 		cat >"$notmuch_counter_command" <<EOF || return
> > > #!/bin/sh
> > > 
> > > read count < "$notmuch_counter_state_path"
> > 
> > Nice.  Fixed in the new patch version.
> > 
> > > echo \$((count + 1)) > "$notmuch_counter_state_path"
> > > 
> > 
> > I do not think this is really an optimization.  And I find expr more
> > clear than using $(()).  I always have troubles remembering "random
> > special char syntax" (yeah, not a Perl fan :)), prefer human friendly
> > words.
> 
> The (posix) shell command language defines 'Arithmetic Expansion' in
> 
> http://pubs.opengroup.org/onlinepubs/007908799/xcu/chap2.html#tag_001_006_004
> 
> I.e. using format $(( expression )) makes shell doing the arithetic itself
> instead of forking a process (or two!) to do so.
> 

I though expr was a builtin.  Now I agree that it is better to replace
it with $(()), even though I still prefer the expr syntax.

> Normally in this case it is not so big deal (and still it isn't, but...)
> In this  particular case the shell wrapper counting notmuch launches and
> exec'ing it the wrapper could do this without fork(2)ing a single time
> (i.e. keep the process count unchanged compared to execing notmuch
> directly)
> 
> Anyway, many opinions; as far as it works I'm fine with it :) 
> 
> Now that you feel relaxed, check the results of some further
> experimentation ;) :
> 
> excerpt from man strace:
> 
>        -ff         If the -o filename option is in effect,  each  processes
>                    trace  is  written  to  filename.pid  where  pid  is the
>                    numeric process id of each process.
> 
> Executing  rm -f forked.*; strace -ff -o forked bash -c 'echo $(( 5 + 5 ))' 
> 
> will output '10' and create just one 'forked.<pid>' file
> 
> Executing  rm -f forked.*; strace -ff -o forked bash -c 'echo $(expr 5 + 5)' 
> 
> output 10 as expected, but there is now *3* forked.<pid> files !
> 
> bash does not optmize; it forks subshell to execute $(...) and then
> there just works as usual (forks subshell to execute builtin expr))
> 
> Executing  rm -f forked.*; strace -ff -o forked bash -c 'echo $(exec expr 5 + 5)' 
> 
> (the added 'exec' takes off one fork -- just 2 forked.<pid> files appear).
> 
> I did the same tests using dash, ksh & zsh on linux system, and every one
> of these managed to optimize one fork out in the above 3 fork case.
> 

Thanks for details.

Regards,
  Dmitry

> 
> Tomi