search

taoensso / nippy

The fastest serialization library for Clojure
https://www.taoensso.com/nippy
Eclipse Public License 1.0
1.04k stars 60 forks source link

The with-cache macro is marked private #175

Open scarytom opened 3 weeks ago

scarytom commented 3 weeks ago

The with-cache macro is marked as private, but, as the docstring says, users may wish to call it if they are using freeze-to-out! or thaw-from-in!. Is there any harm in making this macro public?

https://github.com/taoensso/nippy/blob/5a9a3913319bde692fb2a50696036b5493e15eb1/src/taoensso/nippy.clj#L972-L976

ptaoussanis commented 3 weeks ago

@scarytom Hi Tom, I'm happy to make this public on the next release - just want to clean up some of the docs, and consider final changes before potentially marking the feature as non-experimental.

I'm curious, could you share a little about your use case for the cache feature?

scarytom commented 3 weeks ago

No problem,

I'm hoping to use nippy to deserialise input-streams directly from S3, so that I don't have to hold all the bytes in memory before deserialisation. I'm using thaw-from-in! for this and, taking the code in the fast-thaw fn as an example, I'm wrapping my input-stream in a new DataInputStream and then wrapping the call to thaw-from-in! in with-cache. If you don't think the cache call is necessary, then I guess there is no issue.

https://github.com/taoensso/nippy/blob/5a9a3913319bde692fb2a50696036b5493e15eb1/src/taoensso/nippy.clj#L1780-L1781

I'm slightly curious why there isn't a thaw function that accepts an input-stream (and a corresponding freeze function that takes an output-stream).

ptaoussanis commented 3 weeks ago

If you don't think the cache call is necessary, then I guess there is no issue.

The call to with-cache is only necessary if you're using the cache feature - otherwise it's presence or absence won't have any affect.

I'm slightly curious why there isn't a thaw function that accepts an input-stream (and a corresponding freeze function that takes an output-stream).

No particular reason, happy to look at a PR if you'd like to add something. Would just note that in general I'd recommend using the standard freeze (with header) unless you have a strong reason not to. The header can be important for portability, esp. if you're dealing with long-lived data.

scarytom commented 3 weeks ago

Well, we aren't using the cache feature yet, so this issue might be moot.

I couldn't immediately see how to call the standard freeze (with header) with an output-stream (nor standard thaw on an input-stream).