Db format changes and use CacheKey in more places #108
Conversation
There was a problem hiding this comment.
A few comments but otherwise looks good!
Re: the change to CacheKey as an inline class and using it everywhere, this is a lot more typesafe but also I think this allocates more in a bunch of places?
For an example, now the cache dumps have the wrapper class everywhere, possibly other places too. Are we okay with that?
...ed-cache-incubating/src/commonMain/kotlin/com/apollographql/cache/normalized/api/CacheKey.kt
Show resolved
Hide resolved
...ncubating/src/commonMain/kotlin/com/apollographql/cache/normalized/api/ApolloCacheHeaders.kt
Show resolved
Hide resolved
...ed-cache-incubating/src/commonMain/kotlin/com/apollographql/cache/normalized/api/CacheKey.kt
Show resolved
Hide resolved
...ed-cache-incubating/src/commonMain/kotlin/com/apollographql/cache/normalized/api/CacheKey.kt
Outdated
Show resolved
Hide resolved
...ed-cache-incubating/src/commonMain/kotlin/com/apollographql/cache/normalized/api/CacheKey.kt
Outdated
Show resolved
Hide resolved
...ting/src/commonMain/kotlin/com/apollographql/cache/normalized/internal/DefaultApolloStore.kt
Outdated
Show resolved
Hide resolved
...ncubating/src/commonMain/kotlin/com/apollographql/cache/normalized/internal/RecordWeigher.kt
Show resolved
Hide resolved
...cache-incubating/src/commonTest/kotlin/com/apollographql/cache/normalized/MemoryCacheTest.kt
Show resolved
Hide resolved
| fun serialize(): String { | ||
| return "$SERIALIZATION_TEMPLATE{$key}" | ||
| return "$SERIALIZATION_TEMPLATE{${keyToString()}}" |
There was a problem hiding this comment.
You can probably remove all of the SERIALIZATION_TEMPLATE stuff now that we have a binary format that can encode CacheKey as a distinct type.
There was a problem hiding this comment.
This is actually used by the IJ Plugin, but yes honestly this doesn't need to be here.
...ng/src/commonMain/kotlin/com/apollographql/cache/normalized/sql/internal/RecordSerializer.kt
Show resolved
Hide resolved
Good point... Not too worried about cache dumps as they're a debug thing, but there's |
All right I've ran the read benchmarks comparing the Normalizer using Strings only vs using CacheKeys, and there was no meaningful difference. |
RecordSerializer)CacheKey(now an inline value class) is used in more APIs instead ofStringfor consistencyApolloStore.trim()(used to be opt-in with theblob2format)Also,
ApolloCacheHeaders.EVICT_AFTER_READ- mostly to simplify the code and APINormalizedCache.remove(pattern: String)propagateErrorsif there are no errors