On 12/20/2012 08:46 PM, larry.mart...@gmail.com wrote: > On Thursday, December 20, 2012 6:17:04 PM UTC-7, Dave Angel wrote: >> <snip> > Of course it's a fragment - it's part of a large program and I was just > showing the relevant parts. But it seems these are methods in a class, or something, so we're missing context. And you use self without it being an argument to the function. Like it's a global. > <snip> > Yes, the code works. I end up with just the rows I want. >> Are you only concerned about speed, not fixing features? > Don't know what you mean by 'fixing features'. The code does what I want, it > just takes too long. > >> As far as I can tell, the logic that includes the time comparison is bogus. > Not at all. > >> You don't do anything there to worry about the value of tup[2], just >> whether some >> item has a nearby time. Of course, I could misunderstand the spec. > The data comes from a database. tup[2] is a datetime column. tdiff comes from > a datetime.timedelta() I thought that tup[1] was the datetime. In any case, the loop makes no sense to me, so I can't really optimize it, just make suggestions. > >> Are you making a global called 'self' ? That name is by convention only >> used in methods to designate the instance object. What's the attribute >> self? > Yes, self is my instance object. self.message contains the string of interest > that I need to look for. > >> Can cdata have duplicates, and are they significant? > No, it will not have duplicates. > >> Is the list sorted in any way? > Yes, the list is sorted by tool and datetime. > >> Chances are your performance bottleneck is the doubly-nested loop. You >> have a list comprehension at top-level code, and inside it calls a >> function that also loops over the 600,000 items. So the inner loop gets >> executed 360 billion times. You can cut this down drastically by some >> judicious sorting, as well as by having a map of lists, where the map is >> keyed by the tool. > Thanks. I will try that.
So in your first loop, you could simply split the list into separate lists, one per tup[0] value, and the lists as dictionary items, keyed by that tool string. Then inside the determine() function, make a local ref to the particular list for the tool. recs = messageTimes[tup[0]] Instead of a for loop over recs, use a binary search to identify the first item that's >= date_time-tdiff. Then if it's less than date_time+tdiff, return True, otherwise False. Check out the bisect module. Function bisect_left() should do what you want in a sorted list. >>> cdata[:] = [tup for tup in cdata if determine(tup)] >> >> >> As the code exists, there's no need to copy the list. Just do a simple >> bind. > This statement is to remove the items from cdata that I don't want. I don't > know what you mean by bind. I'm not familiar with that python function. Every "assignment" to a simple name is really a rebinding of that name. cdata = [tup for tup in cdata if determine(tup)] will rebind the name to the new object, much quicker than copying. If this is indeed a top-level line, it should be equivalent. But if in fact this is inside some other function, it may violate some other assumptions. In particular, if there are other names for the same object, then you're probably stuck with modifying it in place, using slice notation. BTW, a set is generally much more memory efficient than a dict, when you don't use the "value". But since I think you'll be better off with a dict of lists, it's a moot point. -- DaveA -- http://mail.python.org/mailman/listinfo/python-list
