Core: Speed up fill_restrictive item_pool pop loop

[Mysteryem]

What is this fixing or adding?

Items from reachable_items are placed in last-in-first-out order, so items being placed will be towards the end of item_pool, but the iteration to find the item was iterating from the start of item_pool.

Now also uses del instead of .pop() for an additional, tiny, performance increase.

It is unlikely for there to be a noticeable difference in most cases. Only generating with many worlds with a high percentage of progression items and fast access rules is likely to see a difference with this change.

--skip_output generation of 400 template A Hat in Time yamls with progression balancing disabled goes from 76s to 43s (-33s or 43% reduction) for me with this patch. This placed 43200 progression items out of 89974 items total (48% progression items).

A Hat in Time has a large percentage of progression items and has fast access rules. Generation of 400 yamls of a mix of games is probably going to take somewhere more like 30 minutes on average, so saving less than 33 seconds on total generation time in that case is going to be close to negligible.

How was this tested?

Ran generation before and after, the same results were produced.

[NewSoupVi]

I was suspicious at first that this would change generations by filling items in the reverse order or something, but then I thought it through and that doesn't appear to be the case

(For the record, the itempool order is intentional - It is an intended use case that worlds can sort the itempool to fill certain items first/last)

[qwint]

I'm seeing a small timeloss, about 10s/316s, on generating hk yamls (50x my normal yaml which is essentially all-sanity with only choice skips enabled) with this change than without and unsure why (tried a couple other games and didn't see any games that improved too)
code looks fine though which makes it more confusing

[Mysteryem]

I'm seeing a small timeloss, about 10s/316s, on generating hk yamls (50x my normal yaml which is essentially all-sanity with only choice skips enabled) with this change than without and unsure why (tried a couple other games and didn't see any games that improved too) code looks fine though which makes it more confusing

Definitely made sure to cherry pick the changes onto the same commit you were testing against (i.e., did not checkout the PR branch), ensuring that all the same changes from the main branch were present and the same seed was generated (assuming the yaml generates deterministically)?

[qwint]

the two versions i tested against were your branch and your branch with the two commits in the PR reverted, so there shouldn't be a branch difference
I thought i used set seed but don't remember for sure but I generated multiple times on both options (with varying different yamls) and saw a pretty consistent time difference that i mentioned

[Mysteryem]

the two versions i tested against were your branch and your branch with the two commits in the PR reverted, so there shouldn't be a branch difference I thought i used set seed but don't remember for sure but I generated multiple times on both options (with varying different yamls) and saw a pretty consistent time difference that i mentioned

Can you attach the yamls you used?

[qwint]

lol

qwintBug.zip
just copied this N times (now everyone point and laugh at all the skips i disable)

[Mysteryem]

I redid the 400x template AHiT geneartion with --skip_output --seed 1, which placed 43_200 items in progression fill and performed:

• Without this PR: 893_420_548 iterations of item_pool and checks of if pool_item is item (74.7s generation on average).
• With this PR: 39_764_252 iterations of item_pool and checks of if pool_item is item (43.4s generation on average).

This is a difference of 853_656_296 iterations and checks, and about 31.3s. Dividing this 31.3s by 853_656_296 gives 3.67E-8s or 36.7ns per iteration and is check.

The 50x qwintBug.yaml HK generation with --skip_output --seed 1 placed 11_251 items in progression fill and performed:

• Without this PR: 61_503_889 iterations + is checks
• With this PR: 1_805_488 iterations + is checks

This is a difference of 59_698_401 iterations and checks. Multiplying this 59_698_401 by 36.7ns gives 2.19s

In actuality, when I ran 7 generations of the HK yamls without this PR and 7 generations with this PR, I see a 4.56s decrease in generation time with this PR, from 373.89s to 369.23s, so within the right sort of ballpark. I'm not really sure why it would decrease slightly more than was expected by extrapolating the AHiT results, there's probably many things that could be affecting it, and some of those could be internal to the CPython implementation.

I ran everything on Python 3.12.9 on Windows 10, on this PR branch with/without the 2 commits.

[qwint]

unsure what anecdotal issues I was seeing per my previous comment, but code looks reasonable given the assumption that the item is more likely at the end of the itempool,
and ran some debugging prints locally to confirm even in my hk case that assumption holds true an overwhelming majority of the time