Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Is it possible to make the hexStr function faster?

#include <sstream>
#include <iomanip>
#include <stdio.h>

std::string hexStr(unsigned char* data, int len)
{
    std::stringstream ss;
    ss << std::hex;
    for(int i=0;i<len;++i)
        ss << std::setw(2) << std::setfill('0') << (int)data[i];
    return ss.str();
}

int main()
{
   std::string packet = "";
   packet += 0x6b;
   packet += 0x0f;
   std::string hex = hexStr((unsigned char*)packet.c_str(), packet.length());
   printf("%s\n", hex.c_str()); // 6b0f
   return 0;
}
share|improve this question

1 Answer 1

up vote 3 down vote accepted

Yes, but I doubt you'll see any practical difference with such short input.

Two ideas: reduce the number of possible dynamic allocations, and do the conversion yourself with a small lookup table.

You can do both of these by pre-allocating the string container (you know the target size already), then doing the hex conversion manually. Something like this:

constexpr char hexmap[] = {'0', '1', '2', '3', '4', '5', '6', '7',
                           '8', '9', 'a', 'b', 'c', 'd', 'e', 'f'};

std::string hexStr(unsigned char *data, int len)
{
  std::string s(len * 2, ' ');
  for (int i = 0; i < len; ++i) {
    s[2 * i]     = hexmap[(data[i] & 0xF0) >> 4];
    s[2 * i + 1] = hexmap[data[i] & 0x0F];
  }
  return s;
}

Running this on my machine, with a test string that is 128k times 256 chars (from 0 to 255) gives me about 1.1s for your code, and about 0.18s for mine (redirecting to /dev/null to avoid I/O issues).

This should be at least not-worse than your version for small strings - there's no opportunity for the compiler to make a sub-optimal inlining decision on the string stream operators in particular, and (named) RVO will take care of eliding the copy from the return value to the caller's std::string hex.

Small benchmark to test this on (bad quality) pseudo-random 300 char long strings:

#include <sstream>
#include <iomanip>
#include <stdio.h>
#include <ctime>
#include <vector>

std::string hexStr(unsigned char *data, int len)
{
  std::stringstream ss;
  ss << std::hex;
  for (int i = 0; i < len; ++i)
    ss << std::setw(2) << std::setfill('0') << (int)data[i];
  return ss.str();
}

constexpr char hexmap[] = {'0', '1', '2', '3', '4', '5', '6', '7',
                           '8', '9', 'a', 'b', 'c', 'd', 'e', 'f'};

std::string hexStr2(unsigned char *data, int len)
{
  std::string s(len * 2, ' ');
  for (int i = 0; i < len; ++i) {
    s[2 * i]     = hexmap[(data[i] & 0xF0) >> 4];
    s[2 * i + 1] = hexmap[data[i] & 0x0F];
  }
  return s;
}

int main()
{
  srand(time(NULL));
  std::vector<std::string> data;

  for (int i = 0; i < 1000; i++) {
    std::string packet;
    for (int i = 0; i < 300; i++)
      packet += static_cast<char>(rand() % 256);
    data.push_back(packet);
  }
  for (int i = 0; i < 10; i++) {
    clock_t beg = clock();
    for (auto const &s : data) {
      std::string hex = hexStr((unsigned char *)s.c_str(), s.length());
      printf("%s\n", hex.c_str());
    }
    fprintf(stderr, "Elapsed 1: %f\n",
            (clock() - beg) / (1.0 * CLOCKS_PER_SEC));
    beg = clock();
    for (auto const &s : data) {
      std::string hex2 = hexStr2((unsigned char *)s.c_str(), s.length());
      printf("%s\n", hex2.c_str());
    }
    fprintf(stderr, "Elapsed 2: %f\n",
            (clock() - beg) / (1.0 * CLOCKS_PER_SEC));
  }
}

Sample output compiled with clang++ (3.5) -std=c++11 -O3 (doesn't vary too much), stdout redirected to /dev/null:

Elapsed 1: 0.018132
Elapsed 2: 0.001365
Elapsed 1: 0.017858
Elapsed 2: 0.001373
Elapsed 1: 0.018429
Elapsed 2: 0.001313
Elapsed 1: 0.018918
Elapsed 2: 0.001229
Elapsed 1: 0.019134
Elapsed 2: 0.001244
Elapsed 1: 0.018915
Elapsed 2: 0.001239
Elapsed 1: 0.018031
Elapsed 2: 0.001266
Elapsed 1: 0.018051
Elapsed 2: 0.001239
Elapsed 1: 0.017071
Elapsed 2: 0.001315
Elapsed 1: 0.017841
Elapsed 2: 0.001239
share|improve this answer
1  
The real world packets are on average 300 bytes long and I have to convert nearly 1000 of them per second. –  Meysam yesterday
1  
Added a small benchmark with that size of stings, looks ok. If this is really hot, you might want to try with a larger lookup table (256 uint16_ts) to do each char conversion in one operation. Larger data cache footprint, but fewer lookups. But benchmark that on your target CPU & compiler, possibly getting close to noise compared to the string allocation. –  Mat yesterday
1  
Your benchmark is great! I really appreciate the effort you put on this! –  Meysam yesterday

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.