Will multi-threading or other method make my program run faster?

Question

I didn't use multi-threading so far as I didn't need to. But as far as I've read, implementing them will make my program slightly faster than it actually is.

from validate_email import validate_email
import os

# the program is reading each line from "emails.txt" and after it checks each email it will remove duplicates and sort the godd / bad emails

def verify_emails(all_emails_file, all_good_emails_file, all_bad_emails_file):
    with open(all_emails_file) as f: all_emails = f.readlines()

    rs_emails = [elem.strip('\n') for elem in all_emails]
    rs_emails_set = set(rs_emails)  # remove duplicates

    good_emails_file, bad_emails_file = open(all_good_emails_file, 'w+'), open(all_bad_emails_file, 'w+')

    for email in rs_emails_set:
        if validate_email(email, verify=True):
            print >> good_emails_file, email
        else:
            print >> bad_emails_file, email

if __name__ == "__main__":
    clear = lambda: os.system('cls')
    clear()
    try:
        verify_emails("emails.txt", "good_emails.txt", "bad_emails.txt")
    except:
        print "\n\nFile with emails could not be found. Please create emails.txt and run the program again\n\n"

My code is perfectly functional, but when it handles big files ( > 2k rows ) it runs really slow. I'd like to take the best out of it and make it as faster as possible using multi-threading or any other methods.

I'd like when answering, if possible, somebody to explain me whether using multi-threading will help me optimize the program or not. More, I'd like somebody to also explain from his / her experience how can I optimize my code

Answer 1

Performance

As others have pointed out, reading the whole lot into memory is bad for performance. To elaborate a bit on this, it means that you're taking up much more memory than you need to and then performing expensive operations on large data sets. If you use for line in file then it's actually only taking one line at a time, which is much lighter on memory. You also have multiple loops even though you could easily perform all your operations in a single iteration, like this:

def verify_emails(email_path, good_filepath, bad_filepath):
    good_emails = open(good_filepath, 'w+')
    bad_emails = open(bad_filepath, 'w+')
    emails = set()
    with open(email_path) as f:
        for email in f:
            email = email.strip()

I'm using strip() here so that all whitespace (including \n) gets removed.

            if email in emails:
                continue

This tests whether email is already in set and if it is will continue to the next iteration of the loop, ie. the next line in the file. If that passes, then you can just add the email to the set and verify it as you did before.

            emails.add(email)
            if validate_email(email, verify=True):
                print >> good_emails, email
            else:
                print >> bad_emails, email

One important note is that you didn't manually close your files, you should always do this! Ideally you'd use with open(file) as f but it's not as easy to set up here. So instead just remember to close them after the loop.

    good_emails.close()
    bad_emails.close()

Style notes

all_emails_file, all_good_emails_file, all_bad_emails_file are bad names. For a start, if all of them have all_ and _file then what's the point of including those? Names should be clear identifiers of a variable's purpose but that doesn't mean you need to give all the details. Sometimes names just need to distinguish different data from each other. In this case it can be assumed they're all files, and all isn't a good description so it's better to have something like email_path, good_filepath, bad_filepath.

Using print >> good_emails, email is an odd construction. You should instead use good_emails.write(email + '\n'). It's more readable and commonly used.

Avoid using lines like with open(all_emails_file) as f: all_emails = f.readlines(). They're usually a bad idea because people might miss the fact that there's a statement at the end and it just becomes confusing.

I don't understand this:

clear = lambda: os.system('cls')
clear()

why not just call os.system('cls')? If there's a good reason, explain with a comment.

And don't use a bare except, that will catch any error at all that happens in your function. If you're going to do this at the very least catch except Exception as error, because then you can print information from error like error.message and type(error) to find out what actually happened. But really, you should know whether you're expecting a ValueError or an IOError and specifically handle what you expect to happen.

Answer 2

As stated in the comments, multi-threading might not be what you are looking for.

---

I think there is room for improvement on the way you read your file. Currently:

1. you read the whole file into a string all_emails = f.readlines()
2. you remove duplicates rs_emails_set = set(rs_emails) # remove duplicates
3. and you read every element of this array for email in rs_emails_set:

Reading this comment, I strongly recommend you to test the following:

processed_emails = set()
for email in f: 
  if email not in processed_emails:
    # validate email or not
    processed_emails.add(email)

Instead of immediately writing the good and bad emails files, you could store those into 2 lists and write them at once at the very end (removing some I/O with the filesystem as well)

Answer 3

Multithreading is not a silver bullet.

As it is your code is I/O bound and the only way to speed that up further is getting a faster harddrive.

There is one little thing you can do:

In your current code there are 3 distinct phases; reading the file into memory, removing duplicates and validation+output.

What you can do is interleave the phases so you can get to work with output while you are still reading in:

rs_emails_set = Set()

for email in f:
    if email not in rs_emails_set
        continue # we already processed this one
    else
        rs_emails_set.add(email)

    if validate_email(email, verify=True):
        print >> good_emails_file, email
    else:
        print >> bad_emails_file, email

This will mostly save you from copying the entire file into memory before processing but instead processes the emails one at a time as they come in.