fix: cache a JSON string and get a string back - #51
Merged
MikaAK merged 1 commit intoAug 16, 2026
Conversation
`encode/2` checked binaries against `~r/^{.*}$/` and stored the brace-wrapped ones
unencoded, which left `decode/1` guessing what it was looking at. Caching a JSON
string returned a decoded map — a String in, a Map out, silently:
put(:k, ~s({"a": 1})) |> get(:k) #=> {:ok, %{"a" => 1}}
The guess had no way to be right. A binary that happens to look like JSON is
indistinguishable from a serialised value once the shape of the payload is the
only evidence available.
So there is no guess now. Binaries are encoded like every other term, and
`decode/1` keys off the external term format version byte, which every value
this library encodes carries and no raw payload does.
Bytes that are not in that format were written by an earlier version or by
something other than this library, so they keep their previous reading: a raw
JSON string still decodes to a map, a raw digit string to an integer. Existing
Redis and DETS data is unaffected — only values written from here on are
type-stable.
Integers still go to the store as integers, so a store that understands numbers
still sees one, and `Cache.Redis.Hash` field names are untouched.
`decode/1` also stops raising on a binary that is not an encoded term. It used
to reach `binary_to_term/1` for anything that was not digits or brace-wrapped,
which is an ArgumentError on any value another tool wrote into the store.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## perf/adapter-aware-term-encoding #51 +/- ##
===================================================================
Coverage ? 83.15%
===================================================================
Files ? 23
Lines ? 659
Branches ? 0
===================================================================
Hits ? 548
Misses ? 111
Partials ? 0 ☔ View full report in Codecov by Harness. |
MikaAK
added a commit
that referenced
this pull request
Aug 16, 2026
`encode/2` checked binaries against `~r/^{.*}$/` and stored the brace-wrapped ones
unencoded, which left `decode/1` guessing what it was looking at. Caching a JSON
string returned a decoded map — a String in, a Map out, silently:
put(:k, ~s({"a": 1})) |> get(:k) #=> {:ok, %{"a" => 1}}
The guess had no way to be right. A binary that happens to look like JSON is
indistinguishable from a serialised value once the shape of the payload is the
only evidence available.
So there is no guess now. Binaries are encoded like every other term, and
`decode/1` keys off the external term format version byte, which every value
this library encodes carries and no raw payload does.
Bytes that are not in that format were written by an earlier version or by
something other than this library, so they keep their previous reading: a raw
JSON string still decodes to a map, a raw digit string to an integer. Existing
Redis and DETS data is unaffected — only values written from here on are
type-stable.
Integers still go to the store as integers, so a store that understands numbers
still sees one, and `Cache.Redis.Hash` field names are untouched.
`decode/1` also stops raising on a binary that is not an encoded term. It used
to reach `binary_to_term/1` for anything that was not digits or brace-wrapped,
which is an ArgumentError on any value another tool wrote into the store.
MikaAK
added a commit
that referenced
this pull request
Aug 16, 2026
* perf: stop encoding terms for adapters that store them natively
Cache.TermEncoder.encode/2 ran on every put and decode/1 on every get, in
the generic Cache macro, for every adapter. Most adapters store Erlang
terms natively, so the round trip was pure overhead — and the decode
dominated the lookup it was attached to. On a 500k-entry ETS table, a bare
:ets.lookup of a ~10KB body costs 63 ns while binary_to_term of the same
body costs ~29,000 ns.
Adapters now declare how they store values via an optional
native_term_storage?/1 callback. Cache resolves it at compile time from the
adapter module attribute, so there is no runtime branch on the hot path.
ETS, Agent, PersistentTerm, ConCache and Counter store terms; Redis stores
bytes and DETS owns a durable on-disk format, so both keep encoding. The
callback is optional and defaults to encoding, so third-party adapters are
unaffected.
ETS with :rehydration_path also keeps encoding — it dumps the table with
:ets.tab2file/2 and reads it back on the next boot, so it is a durable
format with the same backward-compatibility constraint as DETS.
HashRing and RefreshAhead call the encoder directly, bypassing the macro, so
they resolve the capability against the adapter they wrap. MultiLayer needed
no change: each layer is a full Cache module that owns its own encoding, so a
[ETS, Redis] stack already stores a term in ETS and bytes in Redis.
Benchmarks (500k entries, ~10KB body, 20k iterations):
get/1 29,981 ns -> 6,211 ns (4.8x)
put/2 23,542 ns -> 10,804 ns (2.2x)
The win holds across payload sizes — a small map goes from 2,105 ns to 302 ns.
This also fixes three bugs that were consequences of encoding unconditionally:
* ConCache.get_or_store/3 wrote through ConCache directly, bypassing the
encode in put/3, so a later get/1 raised trying to binary_to_term/1 a
raw term.
* decode/1 used Jason.decode!/1 on any brace-wrapped binary, so a value
like "{oops}" raised Jason.DecodeError on read. It now falls back to
returning the binary unchanged.
* The raw ETS API this adapter exposes (match_object/1, select/1,
tab2list/0, foldl/2) saw opaque encoded binaries instead of the terms
that were put.
PersistentTerm also regains the zero-copy read it exists for.
* fix(test): drain the in-flight refresh task before releasing the global lock
The refresh-ahead lock test released the :global lock immediately after a get
that had just spawned a refresh task. That task was still racing toward
:global.set_lock, so it could reach it after the del_lock, take the freed
lock, and refresh the value the very next assertion expects to be untouched —
failing with "locked:locked_key" where "original" was expected.
The race was always there; making get/1 faster widened the window by getting
the test process to del_lock sooner, and it surfaced on a loaded CI runner.
Wait for the spawned task to lose the race and clean itself up while the lock
is still held, so nothing is in flight when the lock is released.
* fix: keep encoding for strategies that hand values to another node
`Cache.HashRing` rpcs the stored value to the node that owns the key, and
`Cache.MultiLayer` under `broadcast_mode: :replicate` pushes it to the other
nodes' layers. Both were resolving `native_term_storage?` against the adapter
they wrap, so a `Cache.ETS` ring stopped encoding — which changes the wire
format between nodes.
That is only safe if every node upgrades at once. During a rolling deploy the
two versions read each other's writes for the same key: a 0.4.x writer leaves a
`term_to_binary/1` blob that this version hands back to the caller unchanged,
and a value written by this version blows up in the old `binary_to_term/1` when
it happens to be a binary.
So both strategies keep encoding, and `Cache.HashRing` is reverted to what it
does on main. Their format is unchanged from 0.4.x and a mixed-version cluster
stays safe. Node-local adapters are untouched and keep the win.
* test: pin the invariant that makes an upgrade safe
Skipping the encode is only safe where the previous version's bytes cannot
survive to be read by this one — which means the value has to die with the VM
that wrote it. Everything that reaches a disk, a Redis server or another node
keeps the 0.4.x representation.
That reasoning was spread across per-adapter docs, so a new adapter or a new
persistence option could quietly break it and nothing would fail. These two
tests state it directly: durable stores encode, volatile stores do not.
Also covers a Redis key written by an older version, which had no test — the
DETS equivalent did.
* fix: cache a JSON string and get a string back (#51)
`encode/2` checked binaries against `~r/^{.*}$/` and stored the brace-wrapped ones
unencoded, which left `decode/1` guessing what it was looking at. Caching a JSON
string returned a decoded map — a String in, a Map out, silently:
put(:k, ~s({"a": 1})) |> get(:k) #=> {:ok, %{"a" => 1}}
The guess had no way to be right. A binary that happens to look like JSON is
indistinguishable from a serialised value once the shape of the payload is the
only evidence available.
So there is no guess now. Binaries are encoded like every other term, and
`decode/1` keys off the external term format version byte, which every value
this library encodes carries and no raw payload does.
Bytes that are not in that format were written by an earlier version or by
something other than this library, so they keep their previous reading: a raw
JSON string still decodes to a map, a raw digit string to an integer. Existing
Redis and DETS data is unaffected — only values written from here on are
type-stable.
Integers still go to the store as integers, so a store that understands numbers
still sees one, and `Cache.Redis.Hash` field names are untouched.
`decode/1` also stops raising on a binary that is not an encoded term. It used
to reach `binary_to_term/1` for anything that was not digits or brace-wrapped,
which is an ArgumentError on any value another tool wrote into the store.
* fix: make compression_level reachable (#50)
The option was documented and dead. No adapter declares it, so passing it in
`opts:` made `NimbleOptions.validate!/2` reject the whole cache at compile time;
and on the paths where opts resolve at runtime the macro's `@compression_level`
was only read from a compile-time list, so it stayed nil and never reached the
encoder. There was no configuration that produced a compressed value.
It is an encoder concern rather than a store concern, so it now lives on the
`use Cache` line and is taken off the adapter opts before they are validated —
an adapter that forwards its opts to the store would choke on an option that was
never meant for it. The `opts: [compression_level: 6]` spelling keeps working.
Setting it also forces encoding on the adapters that hold terms natively. Asking
for compression is asking for bytes, and silently handing back an uncompressed
term is how this option got lost in the first place.
Strategy adapters encode inside the strategy module, through what they wrap, so
the option cannot reach an encoder from there. They raise at compile time now
instead of accepting it and doing nothing.
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
1 participant
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.
Stacked on #48 — base it back to
mainonce that merges. Independent of #50 (different files).The problem
Cache.TermEncoder.encode/2regex-checked binaries against~r/^{.*}$/and stored the brace-wrapped ones unencoded, while encoding everything else.decode/1then had to work out what it was holding from the shape of the payload:The guess cannot be made correct. Once the payload's shape is the only evidence, a binary that happens to look like JSON is indistinguishable from a serialised value. Anyone caching an API response body, a webhook payload, a rendered JSON blob or a template string hits this, and it fails silently — no error, just a different type on the way out.
The change
There is no guess now.
decode/1keys off the external term format version byte (131), which every value this library encodes carries and no raw payload does.Existing data is untouched
Bytes that are not in external term format were written by an earlier version or by something other than this library, and they keep exactly the reading they have always had — a raw JSON string in Redis still decodes to a map, a raw digit string to an integer. Nothing has to be migrated or re-warmed; only values written from this version on are type-stable.
Two things deliberately left alone so nothing else shifts underneath existing keys:
Cache.Redis.Hashfield names are unchanged.maybe_encode_hash_field/2passes binaries through and only encodes non-binaries, so integer field names still land as"123". An existing hash stays addressable.Also fixed
decode/1no longer raises on a binary that is not an encoded term. It used to fall through to:erlang.binary_to_term/1for anything that was neither digits nor brace-wrapped, which is anArgumentErroron any value written into the store by another tool.Tests
test/cache/json_string_round_trip_test.exs— 10 tests: JSON object strings on Redis and ETS, a brace-wrapped non-JSON string, a digit string staying a string, an integer staying an integer, a binary that itself starts with byte131, an assertion on the stored Redis bytes, and four holding the legacy reading of values written by an earlier version.Not vacuous — 3 of them fail on the branch without the encoder change.
One existing test changed rather than deleted:
&encode/2 encodes JSON properlyassertedjson === encode(json, nil), which pinned the passthrough this PR removes. It now asserts the encoded form round-trips to the same string. The two&decode/1tests covering raw JSON and raw digits are unchanged — renamed to say they cover values written by an earlier version, which is the branch they now exercise.Still asymmetric, on purpose
A raw JSON string handed to
decode/1still comes back as a map. That is the legacy branch, and it is load-bearing for existing keys and for anything an external producer writes into Redis. It could be dropped in a later major once nobody has pre-0.5 values left.