Core: change Region caching to on_change from on-miss-strategy #2366
Conversation
ThePhar
added
is: refactor/cleanup
Berserker66
changed the title
|
It's now failing LttP Dungeon and OoT tests, have not figured out why yet, I assume I missed something that is done to one of the variables I wrapped. This should be slower assuming all worlds used the old API perfectly, but faster in reality, though benchmarking still needs to be done with the latest changes. |
Not sure why is_bunny runs into None regions now, though
should reduce memory footprint at only minor performance change
|
Should be good for testing now. Unittests are passing, it should be faster and more convenient. |
|
300 Factorio worlds: Probably within noise margin. But it's also a world that already behaved in regards to the old API and this PR prevents "mistakes". |
|
Test 1: 3 1-coinsanity DLCQuests because that's what I had in Players #2366: 65.06 seconds If I still remembered how to do t-tests, this is where I would tell you it's not significantly different. Going test ER games next (this included full playthrough because I forgot to turn it off) |
|
Test 2: 10 default SDV template yamls #2366: 85.2261703000404 Testing with ER turned on next (but not chaos because no) |
|
You gotta roll with set seed ( |
|
I'm the beta world developer for CrossCode. In a test with 1000 random worlds, when run on main, time spent in create_regions would creep up by a factor of about 160 by the end of the generation. With #2366 merged into the local branch, time spent in the function shot down to an average of 0.0027 seconds as opposed to the 0.1376 seconds average when generated on main (these were tested with the same seed). In short, this makes all the difference for my world. |
Ah, I thought the point was to test it without determinism. I can change my settings then |
|
Test 3: 10 SDV Full ER seeds, seed # 123456 main: 115.04147580009885 seconds Difference appears to be minimal, as predicted, in small worlds. Will test with 100 worlds next |
|
Tests for RoR2: seed # 70986686017450515584 100 world: 300 world: 10 world. |
|
ap-roller with 3 yamls per run and Update: Update2: i see no difference in failure behaviour, so the mentioned games seem to work with the changed code
|
|
After letting it run for a bit longer, the "up to 12 yamls" ended up not being faster on average. I guess that means it very much depends on the game, and most of the games above are not affected. |
|
At this point, I'm wondering what cythonizing the containers would do |
might be worth a shot |
Eventually we want to remove the full recache and just delete them from the cache directly, but I will save that for after ArchipelagoMW#2366
Eventually we want to remove the full recache and just delete them from the cache directly, but I will save that for after ArchipelagoMW#2366
What is this fixing or adding?
should make several parts of AP faster, but have not gotten around to benchmarking yet.
Difference may be negligle, possibly even worse, for small single-world gens.
How was this tested?
unittests and some small gens. SDV is failing unittests currently and I have no yet checked why.