RFC: Zero-copy ByteString creation

[NicolasT]

Hi,

This is mainly a request-for-comments & check whether there's any interest in this, not a real pull request (for now).

Currently the library uses packCStringLen to turn strings returned from the LevelDB API into ByteString. This is an O(n) operation in the length of the string (it results in a memcpy of the string from C heap into Haskell's GC'ed heap).

This can have quite some impact when dealing with large values.

I noticed the library uses unsafeUseAsCStringLen to pass ByteString values to the LevelDB procedures, which doesn't incur any copying, so thought it'd make sense to do the reverse as well.

I think this has one drawback: the values won't show up in memory statistics & profiling, since they're outside the Haskell-managed heap.

I chose to implement the change as contained in this 69c6ff93894 instead of using Data.ByteString.Unsafe.unsafePackCStringFinalizer, since the latter uses Foreign.Concurrent.newForeignPtr to create a ForeignPtr, and its documentation says:

There is no guarantee of promptness, and in fact there is no guarantee that the finalizer will eventually run at all.

(emphasis mine) which makes me kinda nervous. Might want to check with @dons, @dcoutts or someone else more knowledgeable than me :smile:

[NicolasT]

Thinking about it, not all changes from ad4deef are safe: notably the ones on iter aren't (keys & values yielded by an iter are only safe until the next iter modification). For get, things should still be OK thought. For comparators and alike, I assume one isn't supposed to keep given strings across invocations.

[kim]

Somehow I switched to 'unsafeUseAsCStringLen' for writes and then forgot about this PR.

I did not add zero-copy ByteStrings when handing out values, as this seems unsafe. You think this PR can be closed, then?

[kim]

Closing due to bitrot. @NicolasT feel free to reopen