Use [JsonConverter] attribute instead of IJsonWriteable interface to allow custom serialization of PersistentResponse

[KrzysFR]

In the latest master branch, at commit ea60c59 , a new interface IJsonWriteable is introduced to custom serialize a PersistentResponse.

That can be a problem if you are providing your own Json Serializer (via the IJsonSerializer extension point), because now the server code will always use NewtonSoft's JSON.Net to serialize PersistentResponse instances, instead of the provided serializer.

In the particular case of JSON.Net there is a mechanism to allow custom serialization of types via the [JsonConverter] attribute (http://james.newtonking.com/projects/json/help/index.html?topic=html/T_Newtonsoft_Json_JsonConverterAttribute.htm).

I don't have much experience with JSON.Net itself, so I don't know what would be the impact on perfs, but hopefully that would not change much.

I can see that, with custom serialization, the way that PersistentResponse are serialized wouldbecome some sort of contract that people bringing their own serializers will have to follow. I already had to do that when the custom serialization was introduced in Commit 134ca83

I guess the serialized format can be considered a part of the Protocol, so either it is defined somewhere in the wiki, and IJsonStringifier implementors have to follow that, or the doc says "just copy what the default implementation does" and keep up whenever the format is changed ? I don't mind having to do the latter because that's what I did already.

[KrzysFR]

Btw just to add: NewtonSoft JSON.Net and ANTS Performance Profiler are not friend with each other: it is not possible to profile a process that uses JSON.Net in "line-level and method-level" mode, it will die with an Exception. It only works in "method-level" mode, and that makes it really difficult to see what's going on.

Currently, I cannot profile SignalR.Client in line-level mode because it uses Json.NET at the core, but I was able to profile the server by providing a custom profiler.

[davidfowl]

That second point is an issue for JSON.NET not for SignalR

[KrzysFR]

Sure but that's one reason why someone would need to customize the IJsonStringifier.

[davidfowl]

No it's the reason to file a bug on JSON.NET so that they can fix the issue. Working around it isn't fixing it.

[KrzysFR]

Mmm after re-reading the code, it seems that a lot of things have changed since the previous bus, including how and at what time things get serialized, and my mental model is wrong.

It seems that now the actual message payload gets serialized immediately when the message is published on the bus (here https://github.com/SignalR/SignalR/blob/master/SignalR/Connection.cs#L92 ) and so the serialization of the PersistentResponse only needs to merge raw JSON ...

That means that the packet formatting layer should probably not use the same custom IJsonStringifier specified by the user, but maybe either only use JSON.Net, or have a different extension point if one needs to override that part..

Perviously I thought that the whole packet + message payload was serialized in one pass, at the time where the clients gets the data, hence why I need to have handle serializing the PersistentResponse as well.

I will revisit my code knowing that, but maybe there should be a clear split between the serializer that sees the payload objects, and another one to handle the wire format (probably with a different API, like IPacketSerializer.FormatResponse(TextWriter writer, PersistentResponse repsonse) or something.

I guess the initial problem I had to solve is more similar to Issue #672.

[JamesNK]

dotTrace works a-ok with Json.NET. If ANTS Profiler doesn't work then that's its problem.

[KrzysFR]

--This Issue is obsolete since the IJsonWriteable was removed a long time ago.--

Edit: doh! It is called 'IJsonWritable' with no 'e', but anyway discard that, I have opened a new Issue #954 for the real underlying problem that I have.