C++ XOR Function

Question

This XOR function is costing my program too much time (specifically the conversions.)

How can this code be made faster?

---

string xor_str(string astr, string bstr){
    unsigned long a = strtoul(astr.c_str(), NULL, 16);  // Convert strings to longs
    unsigned long b = strtoul(bstr.c_str(), NULL, 16);

    stringstream sstream;
    sstream << setfill('0') << setw(16) << hex << (a ^ b);  // XOR numbers and
    string result = sstream.str();  // Save result          // convert to string
    sstream.clear();  // Clear buffer (will this happen anyway?)
    return result;
}

For example, xor_str("74657374696e6731", "1111111111111111") returns "65746265787f7620".

Answer 1

How about doing the xor on patched ASCII values instead, I'm sort of in doubt about performance of general purpose strtoul.

I didn't profile it, so I may be underestimating the optimizations of modern compilers. Anybody will profile it, just for curiosity? :)

edit: The OP code will work even for input strings with different lengths (i.e. ("1234", "4") will produce 0000000000001230. My code will work only with already leading-zero extended numbers of same length, although the length can be arbitrary, not just 16.

std::string xor_str(const std::string& astr, const std::string& bstr)
{
    const size_t bsize = bstr.size();
    assert(astr.size() == bsize);
    // this xor_str will work over arbitrary long strings, they just have to have same size
    string result(bsize, 0);
    for (size_t i = 0; i < bsize; ++i) {
        // astr[i] and bstr[i] is something from '0'..'9', 'a'..'f' or 'A'..'F'
        char ra = astr[i], rb = bstr[i];
        // make lower nibble of chars 0..F, from ASCII 'a'+9 = 0x6A, 'A'+9 = 0x4A
        if ('9' < ra) ra += 9;
        if ('9' < rb) rb += 9;
        ra = (ra^rb)&0x0F;  // xor lower nibbles of chars
        // transfer 'ra' back to ASCII and store it to result
        if (ra <= 9) result[i] = ra + '0';
        else         result[i] = ra + 'a' - 10; // or 'A' for uppercase output
    }
    return result;
}

Answer 2

Well, obviously you should get rid of all that stringstream stuff in favor of the straightforward

char buffer[100];
sprintf(buffer, "%016x", a ^ b);
return buffer;

That'll cut out all those virtual function calls and stateful stream operations in favor of just doing what you want: print a single integer in hex.

---

Also, you may not have noticed, but you're taking the parameters astr and bstr by value instead of by const reference, so depending on how you're calling this function, you might be spending most of your time making copies of std::string objects, not hexifying them per se. My guess is that this is actually a bigger time sink than the stringstream stuff, even though that's what caught my eye first.

---

Therefore I'd try benchmarking

std::string xor_str(const std::string& astr, const std::string& bstr)
{
    unsigned long a = strtoul(astr.c_str(), NULL, 16);
    unsigned long b = strtoul(bstr.c_str(), NULL, 16);
    char buffer[100];  // "big enough"
    snprintf(buffer, sizeof buffer, "%016lx", a ^ b);
    return buffer;
}

(Notice that you could use sprintf instead, since in this particular case you know there can't be any overflow — and you are concerned about raw speed. But my assumption is that you aren't very good at profiling to find hotspots, and are looking for style tips in general, so the general advice "Always use snprintf over sprintf" wins out over the very case-specific and chainsawish "Omit bounds checks when you're sure they're unnecessary.")