0

currently I'm scripting a heuristic problem in Python and I'm trying to make the script as fast as possible. The script is working and now it's the challenge to optimize it. I want to use multithreading (or multiprocessing) to do a small piece of my whole code, here's the piece of code I'm talking about:

valid_children = set()
for focus_genome in a_list_of_genomes: # a list of objects
    [valid_children.update(set([child])) for child in \ 
    focus_genome.children() if not child in all_genomes]

which is basically the same as, more readable:

valid_children = set()
for focus_genome in a_list_of_genomes: # a list of objects
    children = focus_genome.children()
    for child in children: # a list of objects (again)
        if child in all_genomes:
            valid_children.update(set([child]))

all_genomes is a list with all found genomes yet.

I tried the following but it doesn't update the set:

def threaded_function(focus_genome):
    # same as above
    [valid_children.update(set([child])) for child in focus_genome.children() \
    if not child in all_genomes]

def main_function():
    ...
    do things
    ...
    thread = Thread(target = threaded_function, args = (focus_genome,) )
    thread.start()
    thread.join()

Who can help me get back on the road ? Thanks in advance!

9
  • First, that is very definitely not more readable. Your list comprehension creates a useless list full of None values, just for side effects buried under the covers. Not to mention that it doesn't fit on an 80-column screen (or an SO question) without horizontal scrolling. Commented Jan 7, 2014 at 21:48
  • Second, valid_children.update(set([child])) is a very convoluted way of writing valid_children.add(child). Why are you creating a 1-element list, then converting it to a 1-element set, all just to get that 1 element added to another set? Commented Jan 7, 2014 at 21:50
  • Meanwhile, at least in CPython, using multiple threads is not going to make your code any faster if all of your time in spent executing Python code (as opposed to, say, reading a disk file or an HTTP URL, or executing certain kinds of NumPy functions). Commented Jan 7, 2014 at 21:51
  • Finally, if I try to fill in the gaps as simply as possible, I get the example same results doing thread = Thread(…); thread.start(); thread.join() in the loop instead of putting the code directly inside the loop. So, your problem appears to be in the code that you didn't show us (which I had to guess at), rather than the code that you did. Which means we can't debug it for you. Which is exactly why you should always provide an SSCCE instead of just random fragments of code. Commented Jan 7, 2014 at 21:58
  • For example, run this code, and it will print out {Genome(2), Genome(6)}, proving that it does update the set. Commented Jan 7, 2014 at 21:59

1 Answer 1

2

There are some problems with your code… but not the one you're asking asking about.

Since you didn't provide enough to run anything, I added this extra stuff:

class Genome(object):
    i = 0
    def __init__(self, newi = None):
        if newi is None:
            newi = Genome.i
            Genome.i += 1
        self.i = newi
    def __repr__(self):
        return 'Genome({})'.format(self.i)
    def children(self):
        return self._children

g1, g2 = Genome(), Genome()
g1._children = [Genome(), Genome()]
g2._children = [Genome(), Genome(), Genome()]
a_list_of_genomes = [g1, g2]
all_genomes = [g1.children()[0], g2.children()[2]]

Now, your algorithm should give us genomes #2 and #6. So, let's try your non-threaded code:

valid_children = set()
for focus_genome in a_list_of_genomes: # a list of objects
    for child in focus_genome.children(): # a list of objects (again)
        if child in all_genomes:
            valid_children.update(set([child]))
print(valid_children)

I get {Genome(2), Genome(6)}, which is correct.

Now, your threaded code. Copying and pasting the inner loop body as the function body to make sure it's identical:

def threaded_function(focus_genome):
    for child in focus_genome.children(): # a list of objects (again)
        if child in all_genomes:
            valid_children.update(set([child]))

Running it as you tried:

for focus_genome in a_list_of_genomes: # a list of objects
    t = threading.Thread(target=threaded_function, args=(focus_genome,))
    t.start()
    t.join()

print(valid_children)

I get {Genome(2), Genome(6)}, which is not empty as you claim, and correct, and exactly the same as the non-threaded version.


That being said, you're not actually doing anything useful here—and, if you did, you'd have a problem.

First, join sits around waiting for the background thread to finish. So, there is no benefit in starting a thread and immediately joining it. Instead, you need to start a bunch of threads, then join all the threads. For example:

threads = [threading.Thread(target=threaded_function, args=(focus_genome,))
           for focus_genome in a_list_of_genomes]
for thread in threads:
    thread.start()
for thread in threads:
    thread.join()

But if the threads are doing nothing but running CPU-intensive Python code, this won't help anyway, because the Global Interpreter Lock ensures that only one thread can run Python code at a time. Threads are great when you spend all your time doing I/O (reading files or HTTP URLs), waiting on user interaction, or calling slow functions in a handful of libraries like NumPy that are designed for threading. But for running Python code in parallel, they're not going to speed things up at all. For that, you need processes.

Meanwhile, you've got multiple threads trying to mutate a shared object, without any kind of synchronization. That's a race condition, which will lead to corrupted data. You need to use a lock or other sync object to protect shared data if you want to use threads:

valid_children_lock = Lock()

def threaded_function(focus_genome):
    for child in focus_genome.children(): # a list of objects (again)
        if child in all_genomes:
            with valid_children_lock():
                valid_children.update(set([child]))

And this kind of mutable-shared-data threading gets even worse when you're using processes. If you try to directly share a set between two processes, it sometimes works on Unix, and never on Windows.

If you can reorganize your logic to not use mutable shared data, everything gets a lot easier. One really easy way to do this is to write everything in terms of tasks that take parameters and return values—that is, functions without side-effects. Then you can just use a thread pool or executor to run all those tasks and give you back the results. Which has the added advantage that you run as many tasks at a time as you have workers, queuing the rest up automatically, instead of trying to run all of them at once (which is a lot faster).

Can we do that here? Maybe. If each task returns a set of valid_children found for a given focus_genome, then we can just union together all those sets and get the complete result, right? So:

def threaded_task(focus_genome):
    valid_children = set()
    for child in focus_genome.children(): # a list of objects (again)
        if child in all_genomes:
            valid_children.add(child)
    return valid_children

valid_children = set()
with multiprocessing.Pool() as pool:
    for subset in pool.imap_unordered(threaded_task, a_list_of_genomes):
        valid_children.update(subset)

We could simplify this even further by using a couple of comprehensions:

def threaded_task(focus_genome):
    return {child for child in focus_genome.children() if child in all_genomes}
with multiprocessing.Pool() as pool:
    valid_children = set.union(pool.imap_unordered(threaded_task, a_list_of_genomes))
Sign up to request clarification or add additional context in comments.

1 Comment

Great, thanks (:. Couldn't find the information I was looking for and you just put it in one comment. I'm going to append your piece to my code and hope this will solve my problems. Thanks again!

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.