Fixing memory leak

[kazu-yamamoto]

@ocheron found that session values of ByteString lead memory leak. I also found that session keys (IDs) of ByteString leak memory. 1afb213 fixes it. In my server, this reduced memory usage from 30% to 11%.

b0ba6a9 changes the internal data structure from ByteString to ShortByteString to avoid fragmentation. This improves memory footprint from 11% to 5%. I did both experiments with dbMaxSize = 1,000.

[kazu-yamamoto]

Ping @ocheron
I think this should be urgent.

[vincenthz]

why not move to basement Block Word8 instead of ShortByteString ?

[kazu-yamamoto]

I used ShortByteString because it is available if bytestring is installed.

Would you tell me advantages of Block?

[vincenthz]

@kazu-yamamoto much better and complete APIs : http://hackage.haskell.org/package/basement-0.0.10/docs/Basement-Block.html and breaking from the monomorphic Word8-biased world.

or even the type-level-size'd version http://hackage.haskell.org/package/basement-0.0.10/docs/Basement-Sized-Block.html

[kazu-yamamoto]

@vincenthz How to convert ByteString to Block Word8. I gave a look but did not find how to convert.

[ocheron]

convert from memory should work.

[ocheron]

Additionally, I'm wondering if the copy in handshakeTerminate could be moved earlier in the code flow.
If done before calling setSession, the original sessionId would not be retained by the context.
This needs to be done only for sessionIds parsed from a network packet.

Other than that it looks good.
The bang pattern in let !v = (sd',ref) is not necessary but that was preexisting.

EDIT: when -> before

[vincenthz]

as @ocheron point out convert is the way to go, but it's even possible (I haven't checked this particular one) fromList . BS.unpack would be somewhat optimized (good producer/consumer) or even better fromListN (BS.length bs) (BS.unpack bs)

[kazu-yamamoto]

@ocheron @vincenthz Thanks. Now Block and convert are used.

[kazu-yamamoto]

@ocheron

Additionally, I'm wondering if the copy in handshakeTerminate could be moved earlier in the code flow.
If done before calling setSession, the original sessionId would not be retained by the context.
This needs to be done only for sessionIds parsed from a network packet.

Would you pursue this idea in another issue? We should release version 1.5.1 asap.

[kazu-yamamoto]

I have rebased and merged this PR.