Decompress (follow #210) #211
Conversation
|
Force-pushed with a rebase on top of master |
| size_buffer) | ||
| (Deflate.default ~proof:B.proof_bytes level) | ||
| |> function | ||
| | Ok _ -> Cstruct.of_string @@ Buffer.contents res |
There was a problem hiding this comment.
@dinosaure can we avoid a copy here?
There was a problem hiding this comment.
We can use a Bigstring instead a Bytes in decompress and use Cstruct.of_bigarray then in the best case. We can imagine, we allocate a Cstruct.t with Cstruct.create (Cstruct.len buff) as the result in the beginning (so, at most, the size of the input), but as I said we can do that in the best case only. Sometimes, the result of Deflate can be upper than the input (when the input is too small and when we store the huffman tree and the header). Obviously, it's not a common case but I already seen that.
May be, a good way is to reimplement the Buffer module but with Cstruct.t. Another implementation is to start with a Cstruct.t list, reverse the list at the end and concat all Cstruct.t.
There was a problem hiding this comment.
ok makes sense. Let's merge this one as is, we can optimise if needed later.
| then begin | ||
| Buffer.add_subbytes res output_buffer 0 (used_out t); | ||
| Some (Mstruct.of_string (Buffer.contents res)) | ||
| end else Some (Mstruct.of_string (Buffer.contents res)) |
There was a problem hiding this comment.
Can we avoid a string->bigarray copy here? Mstruct.of_cstruct (Cstruct.of_bigarry x) is pretty cheap, would be better if we could use this.
There was a problem hiding this comment.
We can optimize but the best best case, I think, is to have Mstruct.pick_bigstring.
There was a problem hiding this comment.
Another point is to know exactly the size of the output (so output_size <> None).
|
All green, so merging! |
Changes: * Add `ogit add-commit-parent` to add a new parent to a given commit (mirage/ocaml-git#208, @samoht) * port to jbuilder (mirage/ocaml-git#209, @samoht) * port to mtime 1.0 (mirage/ocaml-git#212, @dinosaure and @samoht) * use Decompress instead of camlzip (mirage/ocaml-git#211, @dinosaure and @samoht)
Better implementation of
inflate/deflatefunction. We allocate only one time the needed buffer (inputandoutput) used toinflateanddeflate. Because these functions is used in a stop the world context, it's safe to use the same global buffer - otherwise, it's needed to have these buffer per serialization/deserialization contexts (and it's this case insirodepac).