Skip to content

Commit

Permalink
Core: Speed up CollectionState.copy() using built-in copy methods
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Mysteryem committed Jul 22, 2024
1 parent 4d1507c commit 06a7184
Showing 1 changed file with 8 additions and 9 deletions.
17 changes: 8 additions & 9 deletions BaseClasses.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from __future__ import annotations

import copy
import itertools
import functools
import logging
Expand Down Expand Up @@ -643,14 +642,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
Expand Down

0 comments on commit 06a7184

Please sign in to comment.