Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Super Metroid: Avoid unnecessary deep copies to improve generation speed. #4130

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Zannick
Copy link
Contributor

@Zannick Zannick commented Nov 2, 2024

What is this fixing or adding?

Super Metroid's copy_mixin function showed up in profile results as 145s cumulative time, out of 639s (73 players). Most of that time is in deepcopy and my analysis showed the majority was spent copying the fields helpers and objectives, of which the latter breaks down further into goals and graph.

helpers just keeps a reference to the smbm, so we don't need to create another copy while we're copying, and graph is remarked as being something that doesn't change. As such, we can avoid making copies of those; helpers can point back at the new copy, and graph can be the same object.

Results:

  • while profiling: 639s -> 412s (~36% improved)
  • with spoiler=1: 213s -> 182s (~15% improved)
  • with spoiler=3: 837s -> 742s (~11% improved)

@lordlou
Please note I did not confirm that the graph referenced by Objectives is not modified somewhere deeply hidden in the code, but since this did not break generation with the base settings I believe it works right.

How was this tested?

Generated with vs without this change on console with basic settings only, and compared spoiler logs. Did not try WebHost or map rando.

@github-actions github-actions bot added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Nov 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-on: peer-review Issue/PR has not been reviewed by enough people yet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant