On 8/26/21 7:31 AM, Anton Ivanov wrote: > On 25/08/2021 22:12, Ilya Maximets wrote: >> On 8/25/21 5:17 PM, [email protected] wrote: >>> From: Anton Ivanov <[email protected]> >>> >>> A JSON object sort order is by definition arbitrary. OVS >>> parser(s) do not care about object order - the result is >>> loaded into a SHASH losing any order on the wire. >>> >>> Having the objects sorted is a performance penalty, especially >>> for large objects like f.e. lflow state. That is represented >>> as {"table_name":{"uuid":data, "uuid":data, "uuid":data}} >>> >>> Sorting in a case like this has no meaning neither to human, >>> nor to computer. >> There is a meaning for both human and computer in some cases. >> >> While sorting by UUIDs doesn't make a lot of sense, I agree, >> having sorted columns inside the database row is important >> for debugging purposes. I'm using something like this: >> sed 's/{"_uuid"/\n{"_uuid"/g' >> very frequently, while inspecting database transactions in >> order to split different rows to different lines, so the >> text editor can work with the result or some other scripts >> can process them in a pipeline. And this command relies on >> a fact that '_uuid' goes first in a row. >> >> Also, the thing that unit tests are not failing with this >> change is a pure luck, because, IIUC, we do have a fair amount >> of tests that looks for exact order in transactions. The >> problem here that order will depend on the CPU architecture, >> because different ways of hash computation will be in use. > > JSON which has a "special" requirement to be sorted is NOT JSON.
There is no requirement, but there is convenience in having them sorted. > > 1. The change do not change transaction order, because order of operations in > transaction is expressed via ARRAYS. Any part which is expressed as an object > is an arbitrary order. I didn't say it changes transaction order, I said that it changes order of columns in rows. > > 2. The change is specific solely to JSON RPC which for some reason someone at > some point decided to have sorted. All other parts of OVS do not use sort. I believe it was done for debugability reasons. The same that I have described. > > 3. There is no benefit to it. Only a penalty which is paid for every > transaction. Every time. As I described previously, there is a benefit. It's not a performance, obviously, but convenience and higher debugability. > >> >> In general, I'd consider this change as a step back in >> debugability. So, unless it provides a huge performance >> benefit, I'd like to not have it. And tests should be >> modified in a way that doesn't require exact order of columns, >> for sure. >> >> I'll give it a shot in a scale test run once I have a chance. >> >> Best regards, Ilya Maximets. >> >> If, however, there are 0.5M such records, it >>> is a subtantial CPU (and latency) penalty. >>> >>> Signed-off-by: Anton Ivanov <[email protected]> >>> --- >>> lib/jsonrpc.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c >>> index c8ce5362e..3b44f73fe 100644 >>> --- a/lib/jsonrpc.c >>> +++ b/lib/jsonrpc.c >>> @@ -802,7 +802,7 @@ jsonrpc_msg_to_string(const struct jsonrpc_msg *m) >>> { >>> struct jsonrpc_msg *copy = jsonrpc_msg_clone(m); >>> struct json *json = jsonrpc_msg_to_json(copy); >>> - char *s = json_to_string(json, JSSF_SORT); >>> + char *s = json_to_string(json, 0); >>> json_destroy(json); >>> return s; >>> } >>> >> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
