MultiServer: speed up location commands #1926
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds optimized pure python wrapper around locations dict Adds optimized cython implementation of the wrapper, saving cpu time and 80% memory use
ThePhar
added
is: bug/fix
black-sliver
force-pushed
the
cython-location-collection
branch
from
dc0aa79 to
e8b0732
Compare
black-sliver
force-pushed
the
cython-location-collection
branch
from
4dd880c to
3ac07a2
Compare
The change has no security implications, but ensures that entries[IndexEntry.start] is always valid.
black-sliver
force-pushed
the
cython-location-collection
branch
from
60dec18 to
825baec
Compare
now with 2x nicer names
black-sliver
force-pushed
the
cython-location-collection
branch
from
825baec to
2ed2163
Compare
Berserker66
approved these changes
Jul 4, 2023
ThePhar
added a commit
to ThePhar/Archipelago
that referenced
this pull request
Jul 4, 2023
This reverts commit b6e78bd.
FlySniper
pushed a commit
to FlySniper/Archipelago
that referenced
this pull request
Nov 14, 2023
* MultiServer: speed up location commands Adds optimized pure python wrapper around locations dict Adds optimized cython implementation of the wrapper, saving cpu time and 80% memory use * Speedups: auto-build on import and build during setup * Speedups: add requirements * CI: don't break with build_ext * Speedups: use C++ compiler for pyximport * Speedups: cleanup and more validation * Speedups: add tests for LocationStore * Setup: delete temp in-place build modules * Speedups: more tests and safer indices The change has no security implications, but ensures that entries[IndexEntry.start] is always valid. * Speedups: add cython3 compatibility * Speedups: remove unused import * Speedups: reformat * Speedup: fix empty set in test * Speedups: use regular dict in Locations.get_for_player * CI: run unittests with beta cython now with 2x nicer names
Jouramie
pushed a commit
to Jouramie/Archipelago
that referenced
this pull request
Feb 28, 2024
* MultiServer: speed up location commands Adds optimized pure python wrapper around locations dict Adds optimized cython implementation of the wrapper, saving cpu time and 80% memory use * Speedups: auto-build on import and build during setup * Speedups: add requirements * CI: don't break with build_ext * Speedups: use C++ compiler for pyximport * Speedups: cleanup and more validation * Speedups: add tests for LocationStore * Setup: delete temp in-place build modules * Speedups: more tests and safer indices The change has no security implications, but ensures that entries[IndexEntry.start] is always valid. * Speedups: add cython3 compatibility * Speedups: remove unused import * Speedups: reformat * Speedup: fix empty set in test * Speedups: use regular dict in Locations.get_for_player * CI: run unittests with beta cython now with 2x nicer names
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is this fixing or adding?
Adds optimized pure python wrapper around locations dict
Adds optimized cython implementation of the wrapper, saving cpu time and 80% memory use
Allows releasing gil during !hint
I'm not sure yet
how to automatically build the _speedups andif we really want it in the root directory. Happy for feedback.How was this tested?
Performance was evaluated using original and new function calls with 1000 slot async dummy data. Testing was done running MultiServer.py. Do we want to add tests for the two implementations?
Pure python implementation
Cython implementation
¹ "a lot" is in the ball park of 600x for get_checked and 6x for get_missing for slots with 1000 locations.
This only saves 0.1ms for each connect to such a blank slot, but that's still a big chunk of the CPU time used during Connect.