Repository navigation
Keep the LRU order of CallbackDataCache when persisting it - #5397
Open
waw666waw666 wants to merge 1 commit into
Open
waw666waw666 wants to merge 1 commit into
waw666waw666 wants to merge 1 commit into
Conversation
Reading persistence_data went through LRUCache.__getitem__ for every keyboard, which marked all keyboards as just used in insertion order. Since Application.update_persistence reads it periodically, the most used keyboards could become the first ones to be evicted. Loading the data also inserted the keyboards in that order. Read the keyboards without touching the LRU order, return them sorted by access time and insert them in that order when loading.
This branch has not been deployed
This file contains hidden or 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
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.
With
arbitrary_callback_dataand a persistence, the keyboards that are used the most can be the first ones evicted fromCallbackDataCache, so their buttons start returningInvalidCallbackData.persistence_databuilds its list fromself._keyboard_data.values(). For acachetools.LRUCache, that goes through__getitem__for every key, which marks each keyboard as just used, in insertion order.Application.update_persistencereads this property on every run, so after each run the LRU order is simply "oldest keyboard first", regardless of how recently it was pressed. The same order is used when loading the data after a restart.Changes proposed in this pull request:
persistence_datawithout updating the LRU order, and return them sorted byaccess_time(least recently used first).access_timeinload_persistence_data, so the order after a restart matches the one before (also for data stored by earlier versions).persistence_data, with and without reloading it.