On 12/30/2013 10:55 AM, Holger Hans Peter Freyther wrote:
Good Morning,
during the 30C3 we had some "broken channels" because the BTS didn't
respond within four seconds to a Channel Activation/Deactivation. I
started to look at it with perf and "snprintf" appears to be called a
lot and did show up in the top.
snprintf is mostly called from within osmo_hexdump and gsm_lchan_name.
Both of these function calls are used in our logging calls throughout
the code.
Would it be worth it to get rid of snprintf in osmo_hexdump ?
Anecdotally I've converted similar code away from snprintf to a more raw
variant resulting in a 7-8 times speed increase of the program whose
main purpose was to hex encode messages.
see attached patch for an example
(it may now truncate the hex dumps at a byte boundary vs at a nibble
when using snprintf if the hex exceeds sizeof(hexd_buff) though)
diff --git a/src/utils.c b/src/utils.c
index 5874077..cc33994 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -107,6 +107,7 @@ int osmo_hexparse(const char *str, uint8_t *b, int max_len)
}
static char hexd_buff[4096];
+static const char hex_chars[] = "0123456789abcdef";
static char *_osmo_hexdump(const unsigned char *buf, int len, char *delim)
{
@@ -115,13 +116,20 @@ static char *_osmo_hexdump(const unsigned char *buf, int len, char *delim)
hexd_buff[0] = 0;
for (i = 0; i < len; i++) {
+ const char *delimp = delim;
int len_remain = sizeof(hexd_buff) - (cur - hexd_buff);
- if (len_remain <= 0)
+ if (len_remain < 3)
break;
- int rc = snprintf(cur, len_remain, "%02x%s", buf[i], delim);
- if (rc <= 0)
- break;
- cur += rc;
+
+ *cur++ = hex_chars[buf[i] >> 4];
+ *cur++ = hex_chars[buf[i] & 0xf];
+
+ while (len_remain > 1 && *delimp) {
+ *cur++ = *delimp++;
+ len_remain--;
+ }
+
+ *cur = 0;
}
hexd_buff[sizeof(hexd_buff)-1] = 0;
return hexd_buff;
