Firstly, it's a good idea to pythonize your code before we begin.
When I ran your code, I saw the time to run calculation as the first thing. Looking into your code, I found that line at the end, then had search your entire program before I understood where start_time was defined.
An important thing with programming is - other programmers shouldn't have to do any hunting to understand your code. They will get frustrated if they have to keep searching through spaghetti to find what they're looking for.
So, to fix that, and because we're writing in Python, it's important to have the standard entry point in your code. There are two reasons for this - first it tells your readers where your code starts "here", and second - if your code employs automated documentors, like Sphinx, it instantiates all the objects in your code before performing reflection to document your code. As is, your code would be executed immediately, which would break that intended functionality.
if __name__ == "__main__":
start_time = time.time()
mylist = []
pool = ThreadPool(8)
results = pool.map(my_function, urls())
pool.close()
pool.join()
print_results()
Also, all your program flow is in one place. I can read this, and know what your code does without having to read all the different functions. I moved the print results into it's own function - as you will likely make changes to the print function as time progresses. If you make a change, and you create an error, the trace-back will point clearly to this function as being the error, instead of the main routine as being the error. This makes it easier and faster to fix bugs when you code. Keep everything in it's own function, this is the Single Responsibility Principle (SOLID).
def print_results():
print("--- %s seconds --- REQUESTS" % (time.time() - start_time))
You will notice I've changed results = pool.map(my_function, urls()) - it's important to reduce variables into what they are - as your url variable returns a list of URLs, why not make it a simple function so it is called in one place without the overhead of maintaining memory space? (... for display)
def urls():
return [
'https://en.wikipedia.org/wiki/Python_(programming_language)',
...
'https://pythonprogramming.net/introduction-to-python-programming/',
]
Another comment with that - as time goes on, you'll add and remove URLs, won't you? Every time you open your code up to make a change, there's a chance you'll inadvertently change the behavior of the code or accidentally break the code. We call this the Open Close Principle (SOLID) - your code should be open for enhancement, but closed for modification. To complete the point - it's best you export your URL list into a text/ini file, and when you need to add/remove URLs, you make the changes there, without the code being modified.
Furthermore, think if your module was part of a large compiled program. If you changed a URL inside the code, it would need to be recompiled. Anything that used that library would need to be recompiled too, making your simple change quite laborious. See the point?
Looking further into your code, we can see that mylist[] is created, but you don't really use it - because everything comes back from the pool.map into results. So let's change your code a little to improve that (... for display):
def fetch_url(url_to_parse):
req = Request(url_to_parse, headers={'User-Agent': 'Mozilla/5.0'})
resp = urlopen(req).read()
return resp
def print_results(results):
print(f"--- {time.time() - start_time} Requests: {len(results)}")
if __name__ == "__main__":
...
print_results(results)
Notice I renamed my_function to fetch_url - my_function doesn't mean anything, but fetch_url does. Make sure you name your functions to match what they do, otherwise other programmers need to read the entire function to understand what it does first.
I've also used f-strings (Python 3.6+) which make print statements much cleaner. Take some time to learn them, they're good.
Now that we've fixed your code, here it is:
from urllib.request import Request, urlopen
from multiprocessing.dummy import Pool as ThreadPool
import time
def urls():
return [
'https://en.wikipedia.org/wiki/Python_(programming_language)',
'https://www.python.org/',
'https://stackoverflow.com/questions/tagged/python',
'https://github.com/python',
'https://realpython.com/python-beginner-tips/',
'https://pythonprogramming.net/introduction-to-python-programming/',
]
def read_url(url_to_parse):
req = Request(url_to_parse, headers={'User-Agent': 'Mozilla/5.0'})
return urlopen(req).read()
def print_results(results):
print(f"--- {time.time() - start_time} Requests: {len(results)}")
if __name__ == "__main__":
start_time = time.time()
pool = ThreadPool(8)
results = pool.map(read_url, urls())
pool.close()
pool.join()
print_results(results)
If you want to make your code faster, it's clear the fetching is the slow part. I notice you're mixing threading concepts and multiprocessor concepts, you should spend some time on understanding the differences. Finally, now that your code is modular, it's easy to drop-in a replacement for the fetch_url function. Here is an example which I tested with your code:
https://stackoverflow.com/questions/3472515/python-urllib2-urlopen-is-slow-need-a-better-way-to-read-several-urls
Hope this helps, Good Luck!
