In S3 and SIEVE, don't evict when updating an entry #201
Milestone
Comments
masklinn
added a commit
to masklinn/uap-python
that referenced
this issue
Mar 26, 2024
`*Result` objects are immutable, thus if a `PartialResult` gets filled further it has to be re-set into the cache. This does not change the cache size, but because the current S3 and SIEVE implementations unconditionally check the cache size on `__setitem__` they may evict an entry unnecessarily. Fix that: if there is already a valid cache entry for the key, just update it in place instead of trying to evict then creating a brand new entry. Also update the LRU to pre-check for size (and presence as well), this may make setting a bit more expensive than post-check but it avoids "wronging" the user by bypassing the limit they set. Fixes ua-parser#201
masklinn
added a commit
that referenced
this issue
Mar 26, 2024
`*Result` objects are immutable, thus if a `PartialResult` gets filled further it has to be re-set into the cache. This does not change the cache size, but because the current S3 and SIEVE implementations unconditionally check the cache size on `__setitem__` they may evict an entry unnecessarily. Fix that: if there is already a valid cache entry for the key, just update it in place instead of trying to evict then creating a brand new entry. Also update the LRU to pre-check for size (and presence as well), this may make setting a bit more expensive than post-check but it avoids "wronging" the user by bypassing the limit they set. Fixes #201
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The caching protocol is such that to update a partial result with more domains it has to be re-set.
While the LRU implementation evicts after insertion, the S3 and SIEVE insertions first make room for an entry, then insert the new entry.
For partial entry this is unnecessary, and can create odd states where there are multiple entries in the fifo with different data but the same key. Instead, since the cache entries are mutable (unlike the results) the
valueshould simply be updated in place. The hit has already been recorded since the__getitem__which retrieved the partial entry.Beware: in S3, do not confuse a cache hit with a ghost cache hit, the second should not suppress the insertion.
The text was updated successfully, but these errors were encountered: