Decoding custom packets with Python using Twisted

This server require fast decoding of packets from clients, as I am expecting 5000 concurrent clients that is sending packets possibly every second. This is a game server (5000 players walking send packets each n every step).

I want to see if my implementation is fast enough or there are still room for improvements. The encryption isn't on this code as I only want to tackle the decryption for now.

The decode function is taken from a JavaScript implementation, This is the best output I got after trying to translate it. (though the translation is correct because the results are same from JavaScript and now Python)

def dataReceived(self, data):
    packet = decode(data)
    (counter, packet_id, username, password, unknown) = unpack('<B H 16s 16s 16s', packet)
    print counter
    print packet_id
    print username
    print password
    print unknown

For example the above implementation is pretty straight forward but the concern here is the decode(data) part.

I am new to Python and Twisted so I had to do some maybe absurd way on implementing this.

This is my decode function:

def decode(packet_data):
    cl = 0x6e
    internal_rounds = 2
    i = 0

    seqX = bytearray.fromhex('HEXSTRINGREMOVED')
    seqY = bytearray.fromhex('HEXSTRINGREMOVED')
    result = []

    while i  0:
            if bl <= dl:
                dl -= bl
            else:
                bl = (~bl) & 0xFF
                bl += 1
                dl += bl

            dl ^= bl2
            j -= 1
        if al <= dl:
            dl -= al
        else:
            al = (~al) & 0xFF
            al += 1
            dl += al
        dl ^= dl2
        dl ^= cl
        result.append(dl)
        cl ^= dl
        i += 1

    return "".join(map(chr, result))

I did this dl = ord(packet_data[i]) because I keep getting int -= char error.

Also did this return "".join(map(chr, result)) because I kept getting errors with unpack because the return was previously return result wh

It is hard to review something when you can't test whether your reviewed code actually works, so with that disclaimer lets see what we can do with your code.

Regarding style issues, your variable names are not good, and doesn't convey any useful informations. But leave that as it is...

Here are some elements I've incorporated in the refactoring below:

• Your code sets j=internal_rounds - 1 which in effect sets j=1, so that the while loop never executes more than once. In other words it can be removed

• You calculate i2 and i2 +1 twice for each loop, which could be simplified to some addition if we add a few more variables. That is i is expanded into i2 and i2_1, in the hope that 3 additions on those is cheaper than 3 additions and 4 multiplications. :)

• Several of your steps change a variable temporary before using the result to change the dl variable, so I've compressed those a little

• Remove the calculation of the loop limit for every loop

Is my refactored code faster than yours? You've got to test, as I can't. The same applies for the correctness, but I think it should be the same.

def decode(packet_data):
    i = 0
    i2 = 0
    i2_1 = 1
    packet_length = len(packet_data) - 4

    seqX = bytearray.fromhex('HEXSTRINGREMOVED')
    seqY = bytearray.fromhex('HEXSTRINGREMOVED')
    result = []

    cl = 0x6e

    while i < packet_length:
        dl = ord(packet_data[i])
        bl = seqX[i2_1]
        al = seqX[i2]
        bl2 = seqY[i2_1]
        dl2 = seqY[i2]

        dl += -bl if bl <= dl else (~bl & 0xFF + 1)
        dl ^= bl2

        dl = -al if al <= dl else (~al & 0xFF + 1)
        dl ^= dl2 ^ cl

        result.append(dl)
        cl ^= dl

        i += 1
        i2 += 2
        i2_1 += 2

    return "".join(map(chr, result))

Another micro-change you could try out, is to change the handling of result to:

result = ''
...
result += chr(dl)
...
return result

It could be faster (or slower), as it skips the array handling and the join/map operations, but it uses a string concatenation instead...

StackExchange Code Review Q#56343, answer score: 3