Return-Path: <tomi.ollila@nixu.com>
X-Original-To: notmuch@notmuchmail.org
Delivered-To: notmuch@notmuchmail.org
Received: from localhost (localhost [127.0.0.1])
	by olra.theworths.org (Postfix) with ESMTP id 3AAAA429E21
	for <notmuch@notmuchmail.org>; Tue, 29 Nov 2011 04:58:21 -0800 (PST)
X-Virus-Scanned: Debian amavisd-new at olra.theworths.org
X-Spam-Flag: NO
X-Spam-Score: 0
X-Spam-Level: 
X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none]
	autolearn=disabled
Received: from olra.theworths.org ([127.0.0.1])
	by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)
	with ESMTP id JYUn4HT4HM-0 for <notmuch@notmuchmail.org>;
	Tue, 29 Nov 2011 04:58:17 -0800 (PST)
Received: from taco2.nixu.fi (taco2.nixu.fi [194.197.118.31])
	(using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))
	(No client certificate requested)
	by olra.theworths.org (Postfix) with ESMTPS id CBE86431FB6
	for <notmuch@notmuchmail.org>; Tue, 29 Nov 2011 04:58:16 -0800 (PST)
Received: from taco2.nixu.fi (taco2.nixu.fi [194.197.118.31])
	by taco2.nixu.fi (8.14.3/8.14.3/Debian-5+lenny1) with ESMTP id
	pATCw0qp021194; Tue, 29 Nov 2011 14:58:01 +0200
From: Tomi Ollila <tomi.ollila@iki.fi>
To: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>,
	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: <87hb1ovsz4.fsf@gmail.com>
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>
User-Agent: Notmuch/0.10+15~gb5803e9 (http://notmuchmail.org) Emacs/23.3.1
	(i686-pc-linux-gnu)
X-Face: HhBM'cA~<r"^Xv\KRN0P{vn'Y"Kd;zg_y3S[4)KSN~s?O\"QPoL
	$[Xv_BD:i/F$WiEWax}R(MPS`^UaptOGD`*/=@\1lKoVa9tnrg0TW?"r7aRtgk[F
	!)g;OY^,BjTbr)Np:%c_o'jj,Z
Date: Tue, 29 Nov 2011 14:58:00 +0200
Message-ID: <yf6mxbfxezr.fsf@taco2.nixu.fi>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
X-BeenThere: notmuch@notmuchmail.org
X-Mailman-Version: 2.1.13
Precedence: list
List-Id: "Use and development of the notmuch mail system."
	<notmuch.notmuchmail.org>
List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,
	<mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>
List-Archive: <http://notmuchmail.org/pipermail/notmuch>
List-Post: <mailto:notmuch@notmuchmail.org>
List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>
List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,
	<mailto:notmuch-request@notmuchmail.org?subject=subscribe>
X-List-Received-Date: Tue, 29 Nov 2011 12:58:21 -0000

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.

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.


Tomi