Add bulk key existence checking #25
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
Every usage of `concatenatedPath(key:)` was passing in the `.value` of a `CacheKey`. This introduces the potential for mis-use, if one callsite were to accidentally pass in the `.rawValue` instead. This can be mitigated by passing the `CacheKey` itself into the method, and then only having a single point of failure. This simplifies usage, because the caller no longer needs to know the specifics of how the URL is constructed.
The prior implementation was an `O(n)` algorithm, as it required loading every single `CacheKey` from disk, and then doing a linear search through the resulting array for the proper key. This can be improved to `O(1)` by bypassing that unnecessary work and looking for the key directly.
Adding default implementations for these methods makes `StorageEngine` adoption simpler. Also, correct a typo in documentation.
Add a method to check the existence of multiple keys simultaneously. This allows for more performant checking of multiple keys, instead of iteratively calling `storage.keyExists(_:)`
mergesort
reviewed
Oct 20, 2023
Without a default implementation, new implementations are required to implement this, encouraging better performance than this O(n) implementation
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.
When dealing with importing large amounts of data, it's helpful to be able to preemptively check for the existence of multiple keys before beginning importation.
This adds a single method to
StorageEngine:func keysExist(_ keys: [CacheKey]) async -> [CacheKey].A default implementation does the sensible thing (retrieve
allKeys()and then filter the parameter based on that), butSQLiteStorageEngineoverrides it for storage-specific implementation details, to avoid loading every single key into memory.This patch also contains other minor related improvements (documentation, performance improvements, etc).