git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alex Riesen <raa.lkml@gmail.com>
To: Wincent Colaiuta <win@wincent.com>
Cc: David Kastrup <dak@gnu.org>, Miles Bader <miles@gnu.org>,
	Pierre Habouzit <madcoder@debian.org>,
	Timo Hirvonen <tihirvon@gmail.com>,
	git@vger.kernel.org, Junio C Hamano <junkio@cox.net>
Subject: Re: [PATCH] Make strbuf_cmp inline, constify its arguments and  optimize it a bit
Date: Mon, 8 Oct 2007 00:31:40 +0200	[thread overview]
Message-ID: <20071007223140.GG2765@steel.home> (raw)
In-Reply-To: <EF81F7DD-73C7-4B6F-92D2-4A143CA05365@wincent.com>

Wincent Colaiuta, Mon, Oct 08, 2007 00:12:17 +0200:
> El 7/10/2007, a las 23:54, Alex Riesen escribió:
> 
> >>... All the rest pretty much
> >>was worse than what we started from in that it needed to reevaluate
> >>more conditions and turned out more complicated and obfuscate even to
> >>the human reader.
> >
> >it _is_ smaller. And it is _measurably_ faster on that thing I have at
> >home (and old p4).
> 
> Can we see the numbers and the steps used to obtain them? I'm also a  
> little bit confused about how an inlined function can lead to a  
> smaller executable... or did you just mean lines-of-code?

I did mean the bytes of object code. I never said it produces a
smaller executable.

I compiled with gcc -O2 and -O4, gcc 4.1.2 (Ubuntu 4.1.2-0ubuntu4).
Cut the functions out into their own files and compile them to get the
object code. Compile with -S (assembly) to examine the generated code.
Compare.

#include <stdint.h>
#include <unistd.h>
#include <stdlib.h>
#include <sys/time.h>
#include <stdio.h>
#include <string.h>

struct strbuf {
	size_t alloc;
	size_t len;
	char *buf;
};

int strbuf_cmp2(struct strbuf *a, struct strbuf *b)
{
	int len = a->len < b->len ? a->len: b->len;
	int cmp = memcmp(a->buf, b->buf, len);
	if (cmp)
		return cmp;
	return a->len < b->len ? -1: a->len != b->len;
}

int strbuf_cmp1(struct strbuf *a, struct strbuf *b)
{
	int cmp;
	if (a->len < b->len) {
		cmp = memcmp(a->buf, b->buf, a->len);
		return cmp ? cmp : -1;
	} else {
		cmp = memcmp(a->buf, b->buf, b->len);
		return cmp ? cmp : a->len != b->len;
	}
}

int main(int argc, char *argv[], char *envp[])
{
	struct strbuf s1 = {
		.alloc = 0,
		.len = 50,
		.buf = "01234567890123456789012345678901234567890123456789",
	};
	struct strbuf s2 = {
		.alloc = 0,
		.len = 50,
		.buf = "0123456789012345678901234567890123456789",
	};
	struct strbuf s3 = {
		.alloc = 0,
		.len = 50,
		.buf = "0123456789012345678901234567890123456789012345678x",
	};
	struct timeval tv1, tv2, diff;
	unsigned n;
	int result;
#define CYCLES 0xffffffffu

	strbuf_cmp1(&s1, &s2);
	strbuf_cmp1(&s2, &s3);
	result = 0;
	gettimeofday(&tv1, NULL);
	for (n = CYCLES; n--; ) {
		result += strbuf_cmp1(&s1, &s2);
		result += strbuf_cmp1(&s2, &s3);
		result += strbuf_cmp1(&s1, &s3);
		result += strbuf_cmp1(&s1, &s1);
		result += n;
	}
	gettimeofday(&tv2, NULL);
	timersub(&tv2, &tv1, &diff);
	printf("ph=%ld.%ld (%d)\n", diff.tv_sec, diff.tv_usec, result);

	strbuf_cmp2(&s1, &s2);
	strbuf_cmp2(&s2, &s3);
	result = 0;
	gettimeofday(&tv1, NULL);
	for (n = CYCLES; n--; ) {
		result += strbuf_cmp2(&s1, &s2);
		result += strbuf_cmp2(&s2, &s3);
		result += strbuf_cmp2(&s1, &s3);
		result += strbuf_cmp2(&s1, &s1);
		result += n;
	}
	gettimeofday(&tv2, NULL);
	timersub(&tv2, &tv1, &diff);
	printf("ar=%ld.%ld (%d)\n", diff.tv_sec, diff.tv_usec, result);
	return 0;
}

  reply	other threads:[~2007-10-07 22:31 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-09-24  9:25 mini-refactor in rerere.c Pierre Habouzit
     [not found] ` <1190625904-22808-2-git-send-email-madcoder@debian.org>
     [not found]   ` <1190625904-22808-3-git-send-email-madcoder@debian.org>
2007-09-24 10:38     ` [PATCH 2/2] Make builtin-rerere use of strbuf nicer and more efficient Johannes Schindelin
2007-09-26  0:31     ` Junio C Hamano
2007-09-26  8:41       ` Pierre Habouzit
2007-10-07 14:00   ` [PATCH] Make strbuf_cmp inline, constify its arguments and optimize it a bit Alex Riesen
2007-10-07 14:24     ` Timo Hirvonen
2007-10-07 14:39       ` Pierre Habouzit
2007-10-07 15:46         ` Miles Bader
2007-10-07 16:07           ` David Kastrup
2007-10-07 21:54             ` Alex Riesen
2007-10-07 22:12               ` Wincent Colaiuta
2007-10-07 22:31                 ` Alex Riesen [this message]
2007-10-08  1:45                   ` Miles Bader
2007-10-08  7:23                     ` Pierre Habouzit
2007-10-08  8:54                       ` Florian Weimer
2007-10-08 18:51                         ` Alex Riesen
2007-10-07 16:11         ` Johannes Schindelin
2007-10-07 16:18           ` Timo Hirvonen
2007-10-07 18:25             ` Johannes Schindelin
2007-10-07 16:54           ` Pierre Habouzit
2007-10-07 14:24     ` David Kastrup
2007-10-07 16:10       ` Alex Riesen
2007-10-07 16:27         ` David Kastrup
2007-10-07 21:57           ` Alex Riesen
2007-10-08  2:19             ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20071007223140.GG2765@steel.home \
    --to=raa.lkml@gmail.com \
    --cc=dak@gnu.org \
    --cc=git@vger.kernel.org \
    --cc=junkio@cox.net \
    --cc=madcoder@debian.org \
    --cc=miles@gnu.org \
    --cc=tihirvon@gmail.com \
    --cc=win@wincent.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).