Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow passing a context to deserialize/serialize so that custom aliases can access the context #1078

Open
jpg85 opened this issue Dec 11, 2020 · 4 comments

Comments

@jpg85
Copy link

jpg85 commented Dec 11, 2020

When trying to write a custom type mapping, it would be helpful to pass a context along with the value to be deserialized/serialized.

Presently I'm trying to use Bond to create an RPC for an object oriented language. When serializing objects, I can create a per client LUT to store the live objects. However, when deserializing, I can't directly access that object on a per-client basis due to the deserialize "get_aliased_value" doesn't allow a context parameter. I'm sure such a context would be helpful for other situations as well.

Suggestion (C++):
bond::aliased_type::type get_aliased_value(T const& value, void* pContext);
void set_aliased_value(T& var, bond::aliased_type::type val, void* pContext);

Similar updates for the other types of alias overrides would be needed as well plus equivalents for other languages.

@chwarr
Copy link
Member

chwarr commented Dec 11, 2020

It seems like you want to be able to pool objects, intern them in some per-client scope when transforming them into your custom C++ types. And, it's the per-client scoping (compared to global scoping) that needs to context. I could also see something that uses per-client state to handle the conversion. For example, looking up the client's culture to know how to parse a numerical value in a string into a C++ type. Am I following?

I'm not opposed to adding a void* context parameter to the alias functions and preferentially invoking get_aliased_value(T const& value, void* userContext); &c. if they exist.

Instead of adding a bunch of Serialize and Deserialize overloads with a context parameter, if something like there were implemented, I think it should be limited to just the "low level" Apply function.

Maybe the transforms can have an optional ctor added that takes a context parameter and none of the driving APIs need to be touched... This is not something that most people will use. (And, if it become super popular, adding such overloads is easy to do.)

template <typename Protocols = BuiltInProtocols, typename T, typename Writer>
void SerializeWithContext(const T& t, Writer& output)
{
    void* context = GetCurrentContextViaMagic();
    Apply<Protocols>(
        Serializer<Writer, Protocols>(output, context),
        obj);
}

One potential way I can think to implement this today is to use thread-local storage to store the context pointer. Thread-local storage has drawbacks, for sure, but maybe it will work for you.

I don't think an allocator will work to smuggle such context around: it isn't passed to the aliasing functions. (Maybe it should be...)

I would implement this in each language individually. This isn't a feature that needs to exist in all languages for them to interoperate with each other.

@jpg85
Copy link
Author

jpg85 commented Dec 11, 2020

It seems like you want to be able to pool objects, intern them in some per-client scope when transforming them into your custom C++ types. And, it's the per-client scoping (compared to global scoping) that needs to context. I could also see something that uses per-client state to handle the conversion. For example, looking up the client's culture to know how to parse a numerical value in a string into a C++ type. Am I following?

Yes, that describes it pretty well. I'm already pulling context information from the RPC call itself pretty much as you describe.

While I could do it in two steps, it would be great to deserialize it directly into the object I want, especially in cases where the objects may be deeply embedded in a structure. I could also probably accomplish it with a specialized Transform that hooks into the built-in serializers, but that is a lot of work compared to having a context.

One potential way I can think to implement this today is to use thread-local storage to store the context pointer. Thread-local storage has drawbacks, for sure, but maybe it will work for you.

I'll experiment with thread local storage. Luckily C++ natively supports the concept now so it won't be as ugly. Of course every thread wouldn't necessary need the context or multiple threads may share the same context, so there is some waste. However, it isn't a bad interim solution.

I don't think an allocator will work to smuggle such context around: it isn't passed to the aliasing functions. (Maybe it should be...)

An allocator seems kind of like a hack. Maybe if it was an allocator local to the serialization it is fine, but if it had to be embedded in the schema/generated structures, I would be opposed to that. Of course if it is local to the serialization, then it is essentially a non-generic context.

I would implement this in each language individually. This isn't a feature that needs to exist in all languages for them to interoperate with each other.

Yes, I think you are correct there. Since the context is going to be specific to an application, it's availability in each language only matters for applications that want the feature in a particular language. For my use case, it only needs to be in C++.

@chwarr
Copy link
Member

chwarr commented Dec 12, 2020

A context parameter isn't something that we currently need at Microsoft, so we have no plans to implement this. You are welcome to contribute, if thread-local storage doesn't meet your needs.

@jpg85
Copy link
Author

jpg85 commented Dec 14, 2020

I'll definitely look into this. If I find a non-intrusive way to add the feature, I'll definitely create the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants