From da005cfee3e55608a7f0ce2bb112ee4c5dfc78db Mon Sep 17 00:00:00 2001 From: Rayson Zhu Date: May 04 2016 03:01:00 +0000 Subject: [PATCH 1/4] remove redundant code --- diff --git a/koji/daemon.py b/koji/daemon.py index 0570faa..558a78a 100644 --- a/koji/daemon.py +++ b/koji/daemon.py @@ -798,8 +798,6 @@ class TaskManager(object): #accept this task) bin_avail = avail.get(bin, [0]) self.logger.debug("available capacities for bin: %r" % bin_avail) - median = bin_avail[(len(bin_avail)-1)/2] - self.logger.debug("ours: %.2f, median: %.2f" % (our_avail, median)) if not self.checkRelAvail(bin_avail, our_avail): #decline for now and give the upper half a chance return False From 6f71be25266fa064385acd8a63c212ab1f854365 Mon Sep 17 00:00:00 2001 From: Rayson Zhu Date: May 04 2016 03:04:49 +0000 Subject: [PATCH 2/4] use Quickselect algorithm to calculate the median of bin_avail This will reduce the average complexity of this step from O(n log n) to O(n) --- diff --git a/koji/daemon.py b/koji/daemon.py index 558a78a..361c920 100644 --- a/koji/daemon.py +++ b/koji/daemon.py @@ -768,8 +768,6 @@ class TaskManager(object): avail = {} for bin in bins.iterkeys(): avail[bin] = [host['capacity'] - host['task_load'] for host in bin_hosts[bin]] - avail[bin].sort() - avail[bin].reverse() for task in tasks: # note: tasks are in priority order self.logger.debug("task: %r" % task) @@ -814,7 +812,7 @@ class TaskManager(object): Check our available capacity against the capacity of other hosts in this bin. Return True if we should take a task, False otherwise. """ - median = bin_avail[(len(bin_avail)-1)/2] + median = koji.util.median(bin_avail) self.logger.debug("ours: %.2f, median: %.2f" % (avail, median)) if avail >= median: return True diff --git a/koji/util.py b/koji/util.py index 80e511f..cd73af3 100644 --- a/koji/util.py +++ b/koji/util.py @@ -31,6 +31,7 @@ import sys import time import ConfigParser from zlib import adler32 +from random import randint try: from hashlib import md5 as md5_constructor @@ -609,3 +610,63 @@ def parse_maven_chain(confs, scratch=False): except ValueError, e: raise ValueError, 'No possible build order, missing/circular dependencies' return builds + + +def partition(array, left, right): + """ + Group an array (ranging from indices left to right) into two parts, + those less than or equal to a certain element, and those greater than or equal to the element. + + Return the index of that element + """ + assert 0 <= left <= right <= len(array) + + # choose a random integer as pivot in [left, right] + pivot = randint(left, right) + #print("left={}, right={}, pivot={}, len(array)={}".format(left, right, pivot, len(array))) + pivot_value = array[pivot] + # move pivot to left + array[pivot] = array[left] + while left < right: + while left < right and array[right] >= pivot_value: + right -= 1 + array[left] = array[right] + while left < right and array[left] <= pivot_value: + left += 1 + array[right] = array[left] + # put pivot_value to current position + array[left] = pivot_value + return left + + +def quick_select(array, n, left = 0, right = None): + """ + Returns the n-th smallest element of list within left..right inclusive + See https://en.wikipedia.org/wiki/Quickselect + Note: After calling this function, the elements in the array will be moved + """ + if right is None: + right = len(array) - 1 + assert 0 <= left <= n <= right <= len(array) + while True: + pivot = partition(array, left, right) + if pivot == n: + return array[pivot] + elif pivot < n: + left = pivot + 1 + else: + right = pivot - 1 + + +def median(array, left = 0, right = None, pick_smaller_one = False): + """ + Return the median of given array within left..right inclusive + If there is an even positive number of elements in the given array, then there are two single middle values. + In this case, instead of returning the mean of the two middle values, this function will return the smaller one + if pick_smaller_one is True, otherwise the bigger one. + Note: After calling this function, the elements in the array will be moved + """ + if right is None: + right = len(array) - 1 + assert 0 <= left <= right <= len(array) + return quick_select(array, left + (right - left) / 2 if pick_smaller_one else left + (right - left + 1) / 2, left, right) diff --git a/tests/test_utils.py b/tests/test_utils.py index 6be5326..48721c7 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -31,3 +31,33 @@ class EnumTestCase(unittest.TestCase): """ Test slice access. """ test = koji.Enum(('one', 'two', 'three')) self.assertEquals(test[1:], ('two', 'three')) + +class MedianCalculationTestCast(unittest.TestCase): + def test_partition(self): + array = [3, 1, 4, 1, 5, 9, 2, 6, 5, 3] + pivot = koji.util.partition(array, 0, len(array) - 1) + pivot_value = array[pivot] + self.assertGreaterEqual(pivot, 0) + self.assertLess(pivot, len(array)) + for i in range(0, pivot): + self.assertLessEqual(array[i], pivot_value) + for i in range(pivot + 1, len(array)): + self.assertGreaterEqual(array[i], pivot_value) + def test_quick_select(self): + array = [3, 1, 4, 1, 5, 9, 2, 6, 5, 3] + nth = 5 + nth_value = koji.util.quick_select(array, nth) + array.sort() + expected_nth = array[nth] + self.assertEqual(nth_value, expected_nth) + + def test_median(self): + array = [3, 1, 4, 1, 5, 9, 2, 6, 5, 3] + median = koji.util.median(array) + array.sort(reverse=True) + expected_median = array[(len(array) - 1) / 2] + self.assertEqual(median, expected_median) + + +if __name__ == '__main__': + unittest.main() From ae042ad55147a133049fb69955bb092aa59bd541 Mon Sep 17 00:00:00 2001 From: Rayson Zhu Date: May 04 2016 03:25:19 +0000 Subject: [PATCH 3/4] calculate available capacity as needed and cache the result --- diff --git a/koji/daemon.py b/koji/daemon.py index 361c920..699d54d 100644 --- a/koji/daemon.py +++ b/koji/daemon.py @@ -764,10 +764,8 @@ class TaskManager(object): elif not bins: self.logger.info("No bins for this host. Missing channel/arch config?") return False - #sort available capacities for each of our bins + # available capacities for each of our bins avail = {} - for bin in bins.iterkeys(): - avail[bin] = [host['capacity'] - host['task_load'] for host in bin_hosts[bin]] for task in tasks: # note: tasks are in priority order self.logger.debug("task: %r" % task) @@ -794,7 +792,13 @@ class TaskManager(object): #see where our available capacity is compared to other hosts for this bin #(note: the hosts in this bin are exactly those that could #accept this task) - bin_avail = avail.get(bin, [0]) + + if bin not in avail: + # if the available capacity for this bin is unknown, we will calculate it and cache the result + # bin must be in bin_hosts because bin is in bins + avail[bin] = [host['capacity'] - host['task_load'] for host in bin_hosts[bin]] + # avail[bin] is not an empty array because bin_hosts[bin] has at least one element + bin_avail = avail[bin] self.logger.debug("available capacities for bin: %r" % bin_avail) if not self.checkRelAvail(bin_avail, our_avail): #decline for now and give the upper half a chance From ba918e9968915a0470db4759d41bf15ce098c1c3 Mon Sep 17 00:00:00 2001 From: Rayson Zhu Date: May 04 2016 03:47:57 +0000 Subject: [PATCH 4/4] fix assertions The "right" parameters of these functions shouldn't be equal to len(array). --- diff --git a/koji/util.py b/koji/util.py index cd73af3..8272177 100644 --- a/koji/util.py +++ b/koji/util.py @@ -619,7 +619,7 @@ def partition(array, left, right): Return the index of that element """ - assert 0 <= left <= right <= len(array) + assert 0 <= left <= right < len(array) # choose a random integer as pivot in [left, right] pivot = randint(left, right) @@ -647,7 +647,7 @@ def quick_select(array, n, left = 0, right = None): """ if right is None: right = len(array) - 1 - assert 0 <= left <= n <= right <= len(array) + assert 0 <= left <= n <= right < len(array) while True: pivot = partition(array, left, right) if pivot == n: @@ -668,5 +668,5 @@ def median(array, left = 0, right = None, pick_smaller_one = False): """ if right is None: right = len(array) - 1 - assert 0 <= left <= right <= len(array) + assert 0 <= left <= right < len(array) return quick_select(array, left + (right - left) / 2 if pick_smaller_one else left + (right - left + 1) / 2, left, right)