Lazily deserialize cache entries

[jonathanhefner]

This adds a cache optimization such that expired and version-mismatched cache entries can be detected without deserializing their values. This optimization is enabled when using cache format version >= 7.1 or a custom serializer.

[casperisfine]

I don't like the idea of extending the interface. I already greatly regret adding dump_compressed.

[casperisfine]

I still don't quite like that idea of an adapter. IMO all this logic should be inside the coder itself.

[jonathanhefner]

SerializerAdapter is a coder with swappable serializer and compressor.

Is the primary concern integration with something like paquito?

What if we (1) rename SerializerAdapter to Cache::Coder, and (2) support specifying either a :coder option OR :serializer and :compressor options for Cache::Store? When :coder is specified, it will be used directly, no Cache::Coder involved. Otherwise, :serializer and :compressor will be used via Cache::Coder, with their default values controlled by the cache format version.

[casperisfine]

Is the primary concern integration with something like paquito?

In part yes. I think if a coder is specified, it should have total control over the serialization, Active Support shouldn't prepend anything to the provided payload etc.

support specifying either a :coder option OR :serializer and :compressor options

I think you touch on something here.

Paquito went the route of taking total control over how the Entry (and its content) is serialized.

You can't combine that with a configurable compressor. But in many case users may only want to control how the Entry#value is serialized.

If both were explicitly distinct I think it would make the code clearer.

So 👍 to experiment that (and sorry for dragging my feet on this, I know you've put a lot of efforts into it, I'm just super worried about the maintainability of all this).

[byroot]

Not a big fan of that _load_payload.

Could we pass both the serializer and compressor? If the payload isn't compressed, we pass compressor = nil.

How does that sound?

[jonathanhefner]

If that is what you prefer, we can do that.

The reason I organized it as _load_payload is the same reason I originally used a thunk: this code is essentially part of load, it is merely delayed. Encapsulating the code in _load_payload allows it to be near to load so they can be understood together.

I've pushed a commit that passes serializer and compressor, and compressor = nil when the payload isn't compressed. In my opinion, the code is disjointed and less clear.

If you can tell me what you dislike about _load_payload, perhaps I can iterate on it. If you dislike the name, I can change it. If you dislike LazyEntry delegating to @coder._load_payload, I can pass (a cached copy of) method(:_load_payload) instead.

[byroot]

If you can tell me what you dislike about _load_payload,

A few things:

• It increase the interface, particularly with something that is public but private _load.
• I like that coder, serializers etc have a simple and standard load/dump, inflate/deflate API.
• It makes the entry and coder reflation two way, I prefer when reference can go only in one direction.

Looking at your last commit now.

[jonathanhefner]

• It increase the interface, particularly with something that is public but private _load.

• I like that coder, serializers etc have a simple and standard load/dump, inflate/deflate API.

These could be solved by passing something like method(:_load_payload) instead of the coder instance.

• It makes the entry and coder reflation two way, I prefer when reference can go only in one direction.

In terms of LazyEntry knowing about Cache::Coder, passing method(:_load_payload) would also abstract that relationship.

In terms of references and lifetimes, LazyEntry must still hold references to call @serializer.load and @compressor.inflate, so there isn't much difference either way.