fs2-data icon indicating copy to clipboard operation
fs2-data copied to clipboard

Add MessagePack item serializer and validator

Open jarmuszz opened this issue 1 year ago • 4 comments

This pull request brings serialization and validation to the msgpack.low module.

Serialization is provided for the outside use through tobinary: Pipe[F, MsgpackItem, Byte] and toNonValidatedBinary: Pipe[F, MsgpackItem, Byte] functions. Akin to how the cbor.low serialization is constructed, serialization without validation can potentialy produce malformed data.

Validation API is also exposed through validated: Pipe[F, MsgpackItem, MsgpackItem].

jarmuszz avatar Aug 18 '24 19:08 jarmuszz

The initial concept of having two serializers was dropped as they ran at a nearly identical time :smiley:.

These are the current benchmarks on i7-8550U:

Benchmark                                       Mode  Cnt     Score    Error  Units
MsgPackItemSerializerBenchmarks.serialize       avgt   10  3337.518 ± 39.333  us/op
MsgPackItemSerializerBenchmarks.withValidation  avgt   10  5107.575 ± 65.148  us/op

Validation seems to slow things down a bit and I think that maybe it is possible to make it a little faster.

jarmuszz avatar Sep 07 '24 11:09 jarmuszz

Thanks a ton for this new contribution. I had a first look at it and left a few comments. But it looks great already. :clap:

satabin avatar Sep 07 '24 12:09 satabin

Hi @jarmuszz. I am not sure anymore what the state of this PR is. Is it ready to be re-reviewed or do you still want to address some comments or improvements?

satabin avatar Oct 15 '24 12:10 satabin

Hi @satabin, Sorry for the confusion and a late reply.

I'll try to address these comments this weekend. I've been trying to combine work with full-time studies which turned out to be more time sinking than I imagined :smiley:. I hope that in the following weeks I'll be working on the fs2-data-msgpack project in a more regular fashion since I'm resigning from the job.

jarmuszz avatar Oct 23 '24 17:10 jarmuszz

Hi @jarmuszz. Sorry I am replying late again. What is the state of this PR? Do you still plan on working on some parts? Or is it ready to be merged when approved?

satabin avatar Jan 13 '25 16:01 satabin

Hi, I think it is ready to be merged if approved.

jarmuszz avatar Jan 13 '25 18:01 jarmuszz