Utilizing Threads:
You are doing a number of things to slow down and serialize your code while the point of threads is to do things in parallel.
for idx in range(splitBy):
byteRange = buildRange(int(sizeInBytes), splitBy)[idx]
bufTh = SplitBufferThreads(url, byteRange)
bufTh.start()
bufTh.join()
dataLst.append(bufTh.getFileData())
In the first line, you generate the full list of byte ranges, but then only use the the range for the specific index you are on. This only needs to be generated once, not every time you start a new chunk.
Then in the next three line, you create the thread, start it, and then immediately wait for it to finish. This means you are never running more than one thread at a time. This implementation will be slower than a single threaded implementation because of the over head involved.
On the last line you get the datafile and append all the bytes into your buffer. This is where the bottle-neck is. Request
is just a class that contains the information describing what should be requested. urlopen
is the function that actually makes the internet connection. Even if you weren't serializing your thread creation and execution, you still would not have a performance gain because the function that will take time is not part of the run method. You want to execute all the code in the run()
method, then just access the resultant data after the thread has terminated.
The body of your main should be like this:
threads = []
byteRanges = buildRange(int(sizeInBytes), splitBy)
for idx in range(splitBy):
t = SplitBufferThreads(url, byteRange)
t.start()
threads.append(t)
# all of the threads are downloading files at this point
for t in threads:
t.join()
# file prep stuff ...
with open(fileName, 'w') as fh:
for t in threads:
fh.write(t.data) #where run() stores the data it read
Edit:
It seems like you need a better understanding of how threads work. When you start a thread (in the main thread), run()
is executed in the context of the thread that was started. The main thread can then continue to execute other tasks while the other thread is executing in the background. Once run()
terminates, nothing else executes in the thread.
The other key point is that the call to urlopen()
is when the download begins to happen. If this is not done in the context of a separate thread, it provides no performance gains. This means it must be called from run()
or a method run()
calls. Otherwise, it will execute in the main thread.
The thread class should be implemented like this:
class SplitBufferThreads(threading.Thread):
""" Splits the buffer to ny number of threads
thereby, concurrently downloading through
ny number of threads.
"""
def __init__(self, url, byteRange):
super(SplitBufferThreads, self).__init__()
self.__url = url
self.__byteRange = byteRange
def run(self):
req = urllib2.Request(self.__url, headers={'Range': 'bytes=%s' % self.__byteRange})
# here is where the file download happens within the context of the thread
self.data = getFileData(req)
def getFileData(self, request):
return urllib2.urlopen(request).read()
buildRange()
is going too much on one line.
lst.append('%s-%s' % (i, int(round(1 + i * value/(numsplits*1.0) + value/(numsplits*1.0)-1, 0))))
Adding to a list, formating a string, and two mathematic equations. The equations are non-trivial and it is not immediately apparent what they are actually doing. Both equations should be broken out into separate functions.
Additionally, the if statement is deciding how the first and second values are calculated. It does not effect the string formating or how the list is mutated.
def buildRange(value, numsplits):
lst = []
for i in range(numsplits):
if i == 0:
first = i
second = int(round(1 + i * value/(numsplits*1.0) + value/(numsplits*1.0)-1, 0))
else:
first = int(round(1 + i * value/(numsplits*1.0),0))
second = int(round(1 + i * value/(numsplits*1.0) + value/(numsplits*1.0)-1, 0))
lst.append("{}-{}".format(first, second))
return lst
Once you do this, it becomes clear that second
is not effected by the if statement. Now, we can make the function even simpler. Replacing the equations greatly improves the readability.
def buildRange(value, numsplits):
lst = []
for i in range(numsplits):
first = i if i == 0 else start(i, value, numsplits)
second = end(i, value, numsplits)
lst.append("{}-{}".format(first, second))
return lst
Naming:
lst
is a bad variable name. We know it is a list from when the variable is initialized, but we don't know what it is meant to store.
buildRange()
returns a list of things, yet the name makes it sound like it only makes one.
In my examples, first
and second
are also bad names, but I hadn't fully understood what the values represented until the code was cleaner.
SplitBufferThreads
is a single thread, it should not be pluralized.
The first argument to buildRange()
is value
. The name of the variable that is passed in for this argument is sizeInBytes
. Which one gives you a better idea about what the code is trying to do?
The Python convention for naming function is underscore_spaced, not camelCase.
You are mixing the meaning of a range and how it is used by the specific library you are using. If you changed the library you are using to make the specific request, you then need to change how you are constructing your ranges or parse out the values from the string.