add feature to register custom userdata handlers.

[adriweb]

Hi,

We've been using this library and needed to extend it to support userdata-based class.
It works fine internally but it would be great it this can be upstreamed!

Let us know what you think.

[adriweb]

I am in the process of writing tests (and updating the existing one), so the Travis failure will be gone soon after I make the check know about the new type id.

In addition, I noticed an issue when several of the same userdata are used in the same table, I'll try to debug that soon - it may be because of some caching happening in the seen table.

[adriweb]

There we go.

It turns out the problem was exactly the same as 1a9f3d7 . I fixed it the same way, although now I'm wondering if there is really no way to add a reference to an already seen userdata - I'm not sure why it doesn't work as is.

[coveralls]

Coverage decreased (-3.1%) to 96.891% when pulling dff6871 on adriweb:userdata into 1d3ebe0 on gvx:master.

[gvx]

Fascinating! I hope you don't mind I'll keep thinking about if I want to merge this for a while. I might prefer to add an API for a more generic extension interface, which would subsume your pull request. This would have the benefit of allowing for plugin modules that serialize functions (like #17) as well as userdata serializers.

I am also concerned about passing the bitser internal functions to handlers via the env argument, because the bitser internals are not really intended to be safe to call outside of bitser itself, and I fear this opens users of bitser up to segfaults and other memory safety issues.

[adriweb]

Sure.
I'd like to know more about what you have in mind for a more generic extension interface, this sounds interesting.

Regarding exposing some internal bits to the callbacks, yes, this feels a bit dirty, although I'm not quite sure there's another way to do that with the current codebase. Some "base" functions could be made stable API wise, I suppose (at least across major version releases), this would make it harder to break things by mistake.

[gvx]

I'm working out the details still, but my thinking is to 1) have extensions specify a type string together with a matcher function to determine which values they should handle 2) have the serializer function of an extention just return a number of values that bitser can handle natively which then get serialized like such: \254 extension-id num-items value (value ...). The deserializer function receives the decoded values as arguments.

This might be a bit slower and less memory-efficient than your patch if extensions are used heavily. On the other hand: extensions wouldn't need to know bitser internals, which also means I can freely refactor those internals without breaking any extensions. And this would also mean less of a risk of extensions accidentally messing up the buffer.

[adriweb]

Alright - when you have a PoC for that, let me know, I can run my benchmarks again :)

[adriweb]

Hi there, any news? :)

[gvx]

Not yet, I have been distracted for personal reasons the last month. I expect to be able to get back in the game shortly. I can't promise anything, but I might have a provisional extension mechanism ready in a week or so!

[adriweb]

No problem, take care!

[gvx]

I've written a prototype for the extension API! Check out the extensions branch, and in particular the tentative documentation, I'd love to hear what you think!

[adriweb]

Hi there 👋

I was wondering if we're still waiting for each other to do something, actually :D
Considering we both made in-line comments, should I do something like trying to benchmark the alternative idea?
Anything else?

Thanks

[gvx]

Hey, I've been busy with other things for a while. If you could benchmark the performance impact you mentioned, that would be great. I'll need to look into what I need to do on my end.

[adriweb]

Ok will do!

[adriweb]

I didn't have time to work on it more before I left the company I was working at but hopefully convinced them to make the benchmarking tool open source soon, so I hope I can continue :)

[gvx]

This PR is no longer applicable as of 74a2359.