Skip to content

Comments

Split input stream according to JSON structure, not by newlines#14

Closed
onnokort wants to merge 1 commit intodeavid:masterfrom
onnokort:master
Closed

Split input stream according to JSON structure, not by newlines#14
onnokort wants to merge 1 commit intodeavid:masterfrom
onnokort:master

Conversation

@onnokort
Copy link
Contributor

For example, jsonrpc4j will not output newlines for each JSON structure it
sends.

@deavid
Copy link
Owner

deavid commented Apr 28, 2015

I understand you're trying to compatibilize bjsonrpc with jsonrpc4j, which is good.
But bjsonrpc was aimed for speed, so '\n' split was a decision to avoid understanding json in the first layer, which should give better performance than your solution.
Also, other jsonrpc libraries are HTTP, and not socket based, so they are relying in HTTP protocol to separate different calls.
And finally, and most important here: you will break client-server compatibility between versions with that change. And that's unacceptable.

I suggest to create this as an option when starting bjsonrpc (as an option to connect, or a library-wide option). And that option should default to "split by lines". This way you (and others) can use this library to interface jsonrpc4j adding a simple line at the start of the program, and the rest of the users won't notice anything.

If you know if the JSONRPC standard says anything about how to split the messages, please tell me.

Thanks for your patch. It looks really good

…lines

Connection objects have a new field 'split' which is a function that will be
used to split up the incoming JSON objects. The default behavior is to split
by newline, but it can be switched to separate the stream by disassembling
into JSON chunks by setting this field to 'Connection.split_by_json'.

This makes bjsonrpc compatible with implementations that do not use newlines
to separate JSON objects, such as jsonrpc4j.
@onnokort
Copy link
Contributor Author

Hey,

I understand you're trying to compatibilize bjsonrpc with jsonrpc4j, which is good.
But bjsonrpc was aimed for speed, so '\n' split was a decision to avoid understanding json in the first layer, which should give better performance than your solution.

Understood. Yes, performance is better with the original code, for execution of 100x your test suite, I have:

  • 38.0s with the original code
  • 44.2s with the JSON-aware splitter
  • 38.0s with the updated PR request that allows configuration (see below)

Also, other jsonrpc libraries are HTTP, and not socket based, so they are relying in HTTP protocol to separate different calls.
And finally, and most important here: you will break client-server compatibility between versions with that change. And that's unacceptable.

Hmm, I was assuming that there is sane JSON structure expected on the wire anyways, so it should not do any harm as long as the JSON splitter works as well. Or are you saying that it will break in the HTTP-specific code path? In that case, I'd suggest that I add a corresponding warning 'JSON splitter won't work with HTTP' to the appropriate docs.

I suggest to create this as an option when starting bjsonrpc (as an option to connect, or a library-wide option). And that option should default to "split by lines".
This way you (and others) can use this library to interface jsonrpc4j adding a simple line at the start of the program, and the rest of the users won't notice anything.

Yes, sounds good, I have just done that and updated the PR accordingly. I am not sure whether style this is in the spirit of your library, though. I also added an option to initialize all Connection objects thusly in the server code. These changes does not seem to impact speed of the newline-splitter at all, at least from my limited tests above.
I ran your test suite successfully in all configurations and I tested a configuration of some of my own Java code talking to a bjsonrpc-server. I didn't test HTTP transport specifically - is there anything I should do?

If you know if the JSONRPC standard says anything about how to split the messages, please tell me.

I was reading into this a bit. I found similar thoughts to my own at http://www.simple-is-better.org/json-rpc/transport_sockets.html, where he goes into splitting by netstrings even. I did not find any reference to THE way how to split. The standard seems to be incomplete here, I guess?

@deavid deavid closed this Apr 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants