From 30f97dd7dead5cad3a9d7a1eecf3737a210098c7 Mon Sep 17 00:00:00 2001
From: Mysteryem <Mysteryem@users.noreply.github.com>
Date: Fri, 9 Aug 2024 13:25:39 +0100
Subject: [PATCH] Core: Speed up CollectionState.copy() using built-in copy
 methods (#3678)

All the types being copied are built-in types with their own `copy()`
methods, so using the `copy` module was a bit overkill and also slower.

This patch replaces the use of the `copy` module in
`CollectionState.copy()` with using the built-in `.copy()` methods.

The copying of `reachable_regions` and `blocked_connections` was also
iterating the keys of each dictionary and then looking up the value in
the dictionary for that key. It is faster, and I think more readable, to
iterate the dictionary's `.items()` instead.

For me, when generating a multiworld including the template yaml of
every world with `python -O .\Generate.py --skip_output`, this patch
saves about 2.1s. The overall generation duration for these yamls varies
quite a lot, but averages around 160s for me, so on average this patch
reduced overall generation duration (excluding output duration) by
around 1.3%.

Timing comparisons were made by calling time.perf_counter() at the start
and end of `CollectionState.copy()`'s body, and summing the differences
between the starts and ends of the method body into a global variable
that was printed at the end of generation.

Additional timing comparisons were made, using the `timeit` module, of
the individual function calls or dictionary comprehensions used to
perform the copying.

The main performance cost was `copy.deepcopy()`, which gets slow as the
number of keys multiplied by the number of values within the
sets/Counters gets large, e.g., to deepcopy a `dict[int, Counter[str]]`
with 100 keys and where each Counter contains 100 keys was 30x slower
than most other tested copying methods. Increasing the number of dict
keys or Counter keys only makes it slower.
---
 BaseClasses.py | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/BaseClasses.py b/BaseClasses.py
index 34e7248415f6..092f330bcbb4 100644
--- a/BaseClasses.py
+++ b/BaseClasses.py
@@ -1,7 +1,6 @@
 from __future__ import annotations
 
 import collections
-import copy
 import itertools
 import functools
 import logging
@@ -719,14 +718,14 @@ def update_reachable_regions(self, player: int):
 
     def copy(self) -> CollectionState:
         ret = CollectionState(self.multiworld)
-        ret.prog_items = copy.deepcopy(self.prog_items)
-        ret.reachable_regions = {player: copy.copy(self.reachable_regions[player]) for player in
-                                 self.reachable_regions}
-        ret.blocked_connections = {player: copy.copy(self.blocked_connections[player]) for player in
-                                   self.blocked_connections}
-        ret.events = copy.copy(self.events)
-        ret.path = copy.copy(self.path)
-        ret.locations_checked = copy.copy(self.locations_checked)
+        ret.prog_items = {player: counter.copy() for player, counter in self.prog_items.items()}
+        ret.reachable_regions = {player: region_set.copy() for player, region_set in
+                                 self.reachable_regions.items()}
+        ret.blocked_connections = {player: entrance_set.copy() for player, entrance_set in
+                                   self.blocked_connections.items()}
+        ret.events = self.events.copy()
+        ret.path = self.path.copy()
+        ret.locations_checked = self.locations_checked.copy()
         for function in self.additional_copy_functions:
             ret = function(self, ret)
         return ret