{"thread":{"id":"18419","subject":"[PATCH] Microoptimize strbuf_cmp","startedAt":"2009-03-19T21:09:32Z","lastAt":"2009-03-19T22:27:32Z","messageCount":3,"participants":["Alex Riesen","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"108586","messageId":"20090319210931.GB31014@blimp.localdomain","threadId":"18419","inReplyTo":null,"subject":"[PATCH] Microoptimize strbuf_cmp","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-03-19T21:09:32Z","receivedAt":"2009-03-19T21:09:32Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Make it inline and cleanup a bit. It is definitely less code\nincluding object code, but it is not always measurably faster\n(but mostly is).\n\nSigned-off-by: Alex Riesen \n---\n\nI have this patch for a long time in my tree and vaguely remember\nsending it once.\n\n strbuf.c | 12 ------------\n strbuf.h | 9 ++++++++-\n 2 files changed, 8 insertions(+), 13 deletions(-)\n\ndiff --git a/strbuf.c b/strbuf.c\nindex 6ed0684..e1f4db7 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -137,18 +137,6 @@ void strbuf_list_free(struct strbuf **sbs)\n \tfree(sbs);\n }\n \n-int strbuf_cmp(const struct strbuf *a, const struct strbuf *b)\n-{\n-\tint cmp;\n-\tif (a->len < b->len) {\n-\t\tcmp = memcmp(a->buf, b->buf, a->len);\n-\t\treturn cmp ? cmp : -1;\n-\t} else {\n-\t\tcmp = memcmp(a->buf, b->buf, b->len);\n-\t\treturn cmp ? cmp : a->len != b->len;\n-\t}\n-}\n-\n void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,\n \t\t\t\t const void *data, size_t dlen)\n {\ndiff --git a/strbuf.h b/strbuf.h\nindex 89bd36e..df95960 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -80,11 +80,18 @@ static inline void strbuf_setlen(struct strbuf *sb, size_t len) {\n extern void strbuf_trim(struct strbuf *);\n extern void strbuf_rtrim(struct strbuf *);\n extern void strbuf_ltrim(struct strbuf *);\n-extern int strbuf_cmp(const struct strbuf *, const struct strbuf *);\n extern void strbuf_tolower(struct strbuf *);\n \n extern struct strbuf **strbuf_split(const struct strbuf *, int delim);\n extern void strbuf_list_free(struct strbuf **);\n+static inline int strbuf_cmp(const struct strbuf *a, const struct strbuf *b)\n+{\n+\tint len = a->len < b->len ? a->len: b->len;\n+\tint cmp = memcmp(a->buf, b->buf, len);\n+\tif (cmp)\n+\t\treturn cmp;\n+\treturn a->len < b->len ? -1: a->len != b->len;\n+}\n \n /*----- add data in your buffer -----*/\n static inline void strbuf_addch(struct strbuf *sb, int c) {\n-- \n1.6.2.1.250.ga1458\n"},{"id":"108601","messageId":"7vvdq56ukb.fsf@gitster.siamese.dyndns.org","threadId":"18419","inReplyTo":"20090319210931.GB31014@blimp.localdomain","subject":"Re: [PATCH] Microoptimize strbuf_cmp","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-19T22:01:40Z","receivedAt":"2009-03-19T22:01:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen writes:\n\n> Make it inline and cleanup a bit. It is definitely less code\n> including object code, but it is not always measurably faster\n> (but mostly is).\n\nThe only in-tree user seems to be rerere, so inlining for that single\ncaller will reduce the object side, but I am not sure if this is a good\nchange in the longer term if we want to encourage the use of strbuf\nlibrary.\n\nThe rewrite of the logic does seem worth doing, though.\n"},{"id":"108607","messageId":"20090319222732.GB8433@blimp.localdomain","threadId":"18419","inReplyTo":"7vvdq56ukb.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Microoptimize strbuf_cmp","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-03-19T22:27:32Z","receivedAt":"2009-03-19T22:27:32Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"It can be less object code and may be even faster, even if at the\nmoment there is no callers to take an advantage of that. This\nimplementation can be trivially made inlinable later.\n\nSigned-off-by: Alex Riesen \n---\n\nJunio C Hamano, Thu, Mar 19, 2009 23:01:40 +0100:\n> Alex Riesen writes:\n> \n> > Make it inline and cleanup a bit. It is definitely less code\n> > including object code, but it is not always measurably faster\n> > (but mostly is).\n> \n> The only in-tree user seems to be rerere, so inlining for that single\n> caller will reduce the object side, but I am not sure if this is a good\n> change in the longer term if we want to encourage the use of strbuf\n> library.\n> \n> The rewrite of the logic does seem worth doing, though.\n\nBut then it is only a half of the micro-optimization. In this case,\nthe cost of call to the function's code is comparable with the change\nof the code.\n\nAnyway, FWIW.\n\n strbuf.c | 13 +++++--------\n 1 files changed, 5 insertions(+), 8 deletions(-)\n\ndiff --git a/strbuf.c b/strbuf.c\nindex 6ed0684..bfbd816 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -139,14 +139,11 @@ void strbuf_list_free(struct strbuf **sbs)\n \n int strbuf_cmp(const struct strbuf *a, const struct strbuf *b)\n {\n-\tint cmp;\n-\tif (a->len < b->len) {\n-\t\tcmp = memcmp(a->buf, b->buf, a->len);\n-\t\treturn cmp ? cmp : -1;\n-\t} else {\n-\t\tcmp = memcmp(a->buf, b->buf, b->len);\n-\t\treturn cmp ? cmp : a->len != b->len;\n-\t}\n+\tint len = a->len < b->len ? a->len: b->len;\n+\tint cmp = memcmp(a->buf, b->buf, len);\n+\tif (cmp)\n+\t\treturn cmp;\n+\treturn a->len < b->len ? -1: a->len != b->len;\n }\n \n void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,\n-- \n1.6.2.1.237.g7206c6\n"}]}