Improve memory swap queueing to prioritze swap-ins (#169)

src/game_objects/page.py

@@ -45,14 +45,14 @@ def in_use(self):
     def in_use(self, value):
         self._in_use = value
 
-    @property
-    def swap_in_progress(self) -> bool:
-        return self._started_swap_at is not None
-
     @property
     def swap_requested(self) -> bool:
         return self._waiting_to_swap or self.swap_in_progress
 
+    @property
+    def swap_in_progress(self) -> bool:
+        return self._started_swap_at is not None
+
     @property
     def swap_percentage_completed(self) -> float:
         return self._swap_percentage_completed
@@ -73,17 +73,18 @@ def request_swap(self, swap_whole_row : bool = False):
         """The method called when the player clicks on the page."""
         self._page_manager.swap_page(self, swap_whole_row)
 
-    def init_swap(self, swapping_from : PageSlot, swapping_to : PageSlot):
+    def init_swap(self, swapping_from : PageSlot):
         """The method called by the page manager to set the swap attributes."""
         self._swapping_from = swapping_from
-        self._swapping_to = swapping_to
         self._waiting_to_swap = True
         self._swap_percentage_completed = 0
 
-    def start_swap(self, current_time: int):
+    def start_swap(self, current_time: int, swapping_to : PageSlot):
         """The method called by the page manager to actually start the swap."""
         self._waiting_to_swap = False
         self._started_swap_at = current_time
+        self._swapping_to = swapping_to
+        swapping_to.page = self
 
     def _update_swap(self, current_time):
         """This method is called at each update. If a swap is in progress, it performs

src/game_objects/page_manager.py

@@ -13,9 +13,10 @@ def __init__(self, stage):
         self._stage = stage
 
         self._ram_slots = []
-        self._swap_slots = []
+        self._disk_slots = []
         self._pages = {}
-        self._swap_queue = Queue()
+        self._swap_in_queue = Queue()
+        self._swap_out_queue = Queue()
 
         self._pages_in_ram_label_xy = (0, 0)
         self._pages_on_disk_label_xy = None
@@ -71,14 +72,14 @@ def setup(self):
 
             for row in range(num_swap_rows):
                 for column in range(num_cols):
-                    swap_slot = PageSlot()
+                    disk_slot = PageSlot()
                     x = self._stage.process_manager.view.width + \
                         column * ram_slot.view.width + column * 5
                     y = self._pages_on_disk_label_xy[1] + \
                         35 + row * ram_slot.view.height + row * 5
-                    swap_slot.view.set_xy(x, y)
-                    self._swap_slots.append(swap_slot)
-            self.children.extend(self._swap_slots)
+                    disk_slot.view.set_xy(x, y)
+                    self._disk_slots.append(disk_slot)
+            self.children.extend(self._disk_slots)
 
     def create_page(self, pid, idx):
         page = Page(pid, idx, self)
@@ -90,11 +91,11 @@ def create_page(self, pid, idx):
                 page_created = True
                 break
         if not page_created:
-            for swap_slot in self._swap_slots:
-                if not swap_slot.has_page:
-                    swap_slot.page = page
+            for disk_slot in self._disk_slots:
+                if not disk_slot.has_page:
+                    disk_slot.page = page
                     page.on_disk = True
-                    page.view.set_xy(swap_slot.view.x, swap_slot.view.y)
+                    page.view.set_xy(disk_slot.view.x, disk_slot.view.y)
                     page_created = True
                     break
         self.children.append(page)
@@ -105,63 +106,52 @@ def swap_page(self, page : Page, swap_whole_row : bool = False):
         if page.swap_requested:
             return
 
-        source_slots = self._swap_slots if page.on_disk else self._ram_slots
-        target_slots = self._ram_slots if page.on_disk else self._swap_slots
-
-        can_swap = False
-        swapping_from = None
-        swapping_to = None
-
-        for source_slot in source_slots:
-            if source_slot.page == page:
-                swapping_from = source_slot
-                break
-        for target_slot in target_slots:
-            if not target_slot.has_page:
-                can_swap = True
-                swapping_to = target_slot
-                break
-        if can_swap:
-            swapping_to.page = page
-
-            queue_swap = bool([page for page in self._pages.values() if page.swap_in_progress])
-            page.init_swap(swapping_from, swapping_to)
-            if queue_swap:
-                self._swap_queue.put(page)
-            else:
-                page.start_swap(self._stage.current_time)
-
-            if swap_whole_row:
-                slots_on_same_row = [
-                    slot
-                    for slot in source_slots
-                    if (
-                        slot.view.y == swapping_from.view.y
-                        and slot != swapping_from
-                    )
-                ]
-                for slot in slots_on_same_row:
-                    if slot.has_page:
-                        self.swap_page(slot.page, False)
+        source_slots = self._disk_slots if page.on_disk else self._ram_slots
+        swap_queue = self._swap_in_queue if page.on_disk else self._swap_out_queue
+
+        swapping_from = next(
+            source_slot for source_slot in source_slots if source_slot.page == page)
+        page.init_swap(swapping_from)
+        swap_queue.put(page)
+
+        if swap_whole_row:
+            slots_on_same_row = [
+                slot
+                for slot in source_slots
+                if (
+                    slot.view.y == swapping_from.view.y
+                    and slot != swapping_from
+                )
+            ]
+            for slot in slots_on_same_row:
+                if slot.has_page:
+                    self.swap_page(slot.page, False)
 
     def delete_page(self, page):
         for ram_slot in self._ram_slots:
             if ram_slot.page == page:
                 ram_slot.page = None
                 break
-        for swap_slot in self._swap_slots:
-            if swap_slot.page == page:
-                swap_slot.page = None
+        for disk_slot in self._disk_slots:
+            if disk_slot.page == page:
+                disk_slot.page = None
                 break
         self.children.remove(page)
         del self._pages[(page.pid, page.idx)]
 
-    def _handle_swap_queue(self):
-        if not self._swap_queue.empty():
-            if not bool([page for page in self._pages.values() if page.swap_in_progress]):
-                page = self._swap_queue.get()
-                page.start_swap(self._stage.current_time)
+    def _handle_swap_queues(self, current_time):
+        swap_in_progress = bool([page for page in self._pages.values() if page.swap_in_progress])
+        empty_ram_slot = next((slot for slot in self._ram_slots if not slot.has_page), None)
+        empty_disk_slot = next((slot for slot in self._disk_slots if not slot.has_page), None)
+
+        if not swap_in_progress:
+            if empty_ram_slot and not self._swap_in_queue.empty():
+                page = self._swap_in_queue.get()
+                page.start_swap(current_time, empty_ram_slot)
+            elif empty_disk_slot and not self._swap_out_queue.empty():
+                page = self._swap_out_queue.get()
+                page.start_swap(current_time, empty_disk_slot)
 
     def update(self, current_time, events):
-        self._handle_swap_queue()
+        self._handle_swap_queues(current_time)
         super().update(current_time, events)

src/tests/game_objects/test_page.py

@@ -53,25 +53,24 @@ def test_swap(self, page_manager):
         page = Page(1, 1, page_manager)
 
         swapping_from = PageSlot()
-        swapping_to = PageSlot()
         swapping_from.page = page
-        swapping_to.page = page
+        swapping_to = PageSlot()
 
         assert not page.swap_requested
         assert not page.swap_in_progress
         assert not page.on_disk
         assert page.swap_percentage_completed == 0
 
-        page.init_swap(swapping_from, swapping_to)
+        page.init_swap(swapping_from)
 
         assert page.swap_requested
         assert not page.swap_in_progress
         assert not page.on_disk
         assert page.swap_percentage_completed == 0
         assert swapping_from.page == page
-        assert swapping_to.page == page
+        assert swapping_to.page == None
 
-        page.start_swap(0)
+        page.start_swap(0, swapping_to)
 
         assert page.swap_requested
         assert page.swap_in_progress
@@ -95,7 +94,7 @@ def test_swap(self, page_manager):
         assert not page.swap_in_progress
         assert page.on_disk
         assert page.swap_percentage_completed == 0
-        assert swapping_from.page is None
+        assert not swapping_from.has_page
         assert swapping_to.page == page
 
     def test_click_when_not_on_disk(self, page_manager, monkeypatch):

src/tests/game_objects/test_page_manager.py

@@ -89,7 +89,7 @@ def test_create_page_on_disk(self, page_manager):
         assert page1.view.y == page2.view.y
         assert page2.view.x > page1.view.x
 
-    def test_swap_page_when_can_swap(self, page_manager):
+    def test_swap_page(self, page_manager):
         pages = []
 
         for i in range(PageManager.get_num_cols() * 2):
@@ -101,6 +101,11 @@ def test_swap_page_when_can_swap(self, page_manager):
         page_manager.swap_page(pages[0])
         assert not pages[0].on_disk
         assert pages[0].swap_requested
+        assert not pages[0].swap_in_progress
+        time += 1
+        page_manager.update(time, [])
+        assert not pages[0].on_disk
+        assert pages[0].swap_requested
         assert pages[0].swap_in_progress
         time += 1000
         page_manager.update(time, [])
@@ -113,6 +118,11 @@ def test_swap_page_when_can_swap(self, page_manager):
         page_manager.swap_page(pages[2])
         assert not pages[2].on_disk
         assert pages[2].swap_requested
+        assert not pages[2].swap_in_progress
+        time += 1
+        page_manager.update(time, [])
+        assert not pages[2].on_disk
+        assert pages[2].swap_requested
         assert pages[2].swap_in_progress
         time += 1000
         page_manager.update(time, [])
@@ -125,6 +135,11 @@ def test_swap_page_when_can_swap(self, page_manager):
         page_manager.swap_page(pages[2])
         assert pages[2].on_disk
         assert pages[2].swap_requested
+        assert not pages[2].swap_in_progress
+        time += 1
+        page_manager.update(time, [])
+        assert pages[2].on_disk
+        assert pages[2].swap_requested
         assert pages[2].swap_in_progress
         time += 1000
         page_manager.update(time, [])
@@ -134,22 +149,6 @@ def test_swap_page_when_can_swap(self, page_manager):
         assert pages[2].view.y == pages[1].view.y
         assert pages[2].view.x < pages[1].view.x
 
-    def test_swap_page_when_cannot_swap(self, page_manager):
-        pages = []
-
-        for i in range(PageManager.get_num_cols() * 2):
-            pages.append(page_manager.create_page(1, i))
-
-        assert pages[PageManager.get_num_cols()].on_disk
-
-        (old_x, old_y) = (pages[PageManager.get_num_cols()].view.x, pages[PageManager.get_num_cols()].view.y)
-
-        page_manager.swap_page(pages[PageManager.get_num_cols()])
-
-        assert pages[PageManager.get_num_cols()].on_disk
-        assert pages[PageManager.get_num_cols()].view.x == old_x
-        assert pages[PageManager.get_num_cols()].view.y == old_y
-
     def test_swap_whole_row(self, page_manager):
         pages = []
 
@@ -162,6 +161,8 @@ def test_swap_whole_row(self, page_manager):
         page_manager.swap_page(pages[0], swap_whole_row=True)
 
         for i in range(PageManager.get_num_cols()):
+            time += 1
+            page_manager.update(time, [])
             time += page_manager.stage.config.swap_delay_ms
             page_manager.update(time, [])
             assert pages[i].on_disk
@@ -175,6 +176,8 @@ def test_swap_whole_row(self, page_manager):
         page_manager.swap_page(pages[0], swap_whole_row=True)
 
         for i in range(PageManager.get_num_cols() - 1):
+            time += 1
+            page_manager.update(time, [])
             time += page_manager.stage.config.swap_delay_ms
             page_manager.update(time, [])
             assert not pages[i].on_disk

src/tests/game_objects/test_process.py

@@ -368,8 +368,8 @@ def test_set_page_to_swap_while_running(self, stage, monkeypatch):
 
         process.use_cpu()
 
-
         stage.page_manager.get_page(1, 0).request_swap()
+        stage.page_manager.update(1, [])
         stage.page_manager.update(1000, [])
         assert stage.page_manager.get_page(1, 0).on_disk == True
 
@@ -389,6 +389,7 @@ def test_set_page_to_swap_before_running(self, stage, monkeypatch):
         process.yield_cpu()
 
         stage.page_manager.get_page(1, 0).request_swap()
+        stage.page_manager.update(1, [])
         stage.page_manager.update(1000, [])
         assert stage.page_manager.get_page(1, 0).on_disk == True
 
@@ -409,11 +410,13 @@ def test_remove_page_from_swap_while_running(self, stage, monkeypatch):
         process.use_cpu()
 
         stage.page_manager.get_page(1, 0).request_swap()
+        stage.page_manager.update(1, [])
         stage.page_manager.update(1000, [])
         process.update(0, [])
         assert process.is_blocked == True
 
         stage.page_manager.get_page(1, 0).request_swap()
+        stage.page_manager.update(1001, [])
         stage.page_manager.update(2000, [])
         process.update(0, [])
 
@@ -829,10 +832,12 @@ def test_blinking_animation_deactivation(self, stage):
 
         process.use_cpu()
         stage.page_manager.get_page(1, 0).request_swap()
+        stage.page_manager.update(1, [])
         stage.page_manager.update(1000, [])
         process.update(1000, [])
 
         stage.page_manager.get_page(1, 0).request_swap()
+        stage.page_manager.update(1001, [])
         stage.page_manager.update(2000, [])
         process.update(2000, [])