ansaurus

Question

Python performance improvement request for winkler

Answer 1

+1 A:

I focused more on optimizing to get more out of Python than on optimizing the algorithm because I don't think that there is much of an algorithmic improvement to be had here. Here are some Python optimizations that I came up with.

(1). Since you appear to be using Python 2.x, change all range()'s to xrange()'s. range() generates the full list of numbers before iterating over them while xrange generates them as needed.

(2). Make the following substitutions for max and min:

start = max(0,i-halflen)

with

start = i - halflen if i > halflen else 0

and

end = min(i+halflen+1,len2)

with

end = i+halflen+1 if i+halflen+1 < len2 else len2

in the first loop and similar ones for the second loop. There's also another min() farther down and a max() near the beginning of the function so do the same with those. Replacing the min()'s and max()'s really helped to reduce the time. These are convenient functions, but more costly than the method I've replaced them with.

(3). Use common1 instead of len(ass1). You've kept track of the length of ass1 in common1 so let's use it rather than calling a costly function to find it again.

(4). Replace the following code:

minlen = min(len1,len2)
for same in xrange(minlen+1):
    if (str1[:same] != str2[:same]):
        break
same -= 1

with

for same in xrange(minlen):
    if str1[same] != str2[same]:
        break

The reason for this is mainly that str1[:same] creates a new string every time through the loop and you will be checking parts that you've already checked. Also, there's no need to check if '' != '' and decrement same afterwards if we don't have to.

(5). Use psyco, a just-in-time compiler of sorts. Once you've downloaded it and installed it, just add the lines

import psyco
psyco.full()

at the top of the file to use it. Don't use psyco unless you do the other changes that I've mentioned. For some reason, when I ran it on your original code it actually slowed it down.

Using timeit, I found that I was getting a decrease in time of about 20% or so with the first 4 changes. However, when I add psyco along with those changes, the code is about 3x to 4x faster than the original.

If you want more speed

A fair amount of the remaining time is in the string's find() method. I decided to try replacing this with my own. For the first loop, I replaced

index = workstr2.find(str1[i],start,end)

with

index = -1
for j in xrange(start,end):
    if workstr2[j] == str1[i]:
        index = j
        break

and a similar form for the second loop. Without psyco, this slows down the code, but with psyco, it speeds it up quite a lot. With this final change the code is about 8x to 9x faster than the original.

If that isn't fast enough

Then you should probably turn to making a C module.

Good luck!

Justin Peel 2010-04-30 06:34:01

These changes improved performance of my test scripts by 25% without pysco. Thanks!

Martlark 2010-05-04 07:57:24

xrange v range does not seem to do much using python 2.6.5

Martlark 2010-05-05 06:04:51

Yeah, maybe so. I haven't really looked at the changes in 2.6.5. Plus, we're not using very big of ranges so it shouldn't have a big effect. The reason to use xrange is doesn't form the whole list in memory before iterating over it. Instead, it generates each element as needed.

Justin Peel 2010-05-05 06:18:23

Answer 2

A:

In addition to everything that Justin says, concatenating strings is expensive - python has to allocate memory for the new string then copy both strings into it.

So this is bad:

ass1 = ''
for i in range(len1):
     ...
    if (index > -1):    # Found common character
        ...
        ass1 = ass1 + str1[i]

It will probably be faster to make ass1 and ass2 lists of characters and use ass1.append(str1[i]). As far as I can see from my quick read of the code the only thing you do with ass1 and ass2 afterwards is to iterate through them character by character so they do not need to be strings. If you did need to use them as strings later then you can convert them with ''.join(ass1).

Dave Kirby 2010-04-30 07:05:55

I tried to make ass1 and ass2 lists and it slowed it down which surprised me. I also tried making workstr1 and workstr2 lists which also slowed it down for me. I really thought that at least one or the other would help, but string concatenation must be quite fast or something.

Justin Peel 2010-04-30 14:40:39

i did not notice any speed up when doing this as well

Martlark 2010-05-03 06:11:34

ansaurus

tags:

views:

answers:

Python performance improvement request for winkler

related questions