After profiling OVSDB insert performance it was found
that some significant portion of its time OVSDB is
calling the function json_clone.
Also, the current usages of json_clone never modify the json,
just keeps it to prevent it to be freed.
With that in mind the struct json, json_create, json_clone
and json_destroy were modified to keep a count of how many
references of the json struct are left. Only when that count
reaches zero the json struct is freed.
The old "json_clone" function was renamed as "json_deep_clone".
Some examples of the performance difference:
In these tests a test table with 4 columns (string, string,
bool, integer) was used. All the tests used "commit block".
*** 50 process each inserting 1000 rows ***
Master OVS
Test Duration 131 seconds
Average Inserts Per second 746.2687 inserts/s
Average Insert Duration 134.1382 ms
Minimal Insert Duration 0.166202 ms
Maximum Insert Duration 489.8593 ms
JSON GC Patch
Test Duration 86 seconds
Average Inserts Per second 1176 inserts/s
Average Insert Duration 82.26761 ms
Minimal Insert Duration 0.165448 ms
Maximum Insert Duration 751.2111 ms
*** 5 process each inserting 10000 rows ***
Master OVS
Test Duration 8 seconds
Average Inserts Per second 7142.857 inserts/s
Average Insert Duration 0.656431 ms
Minimal Insert Duration 0.125197 ms
Maximum Insert Duration 11.93203 ms
JSON GC Patch
Test Duration 7 seconds
Average Inserts Per second 8333.333 inserts/s
Average Insert Duration 0.55688 ms
Minimal Insert Duration 0.143233 ms
Maximum Insert Duration 26.26319 ms
Signed-off-by: Esteban Rodriguez Betancourt <estebarb@hpe.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
To easily allow both in- and out-of-tree building of the Python
wrapper for the OVS JSON parser (e.g. w/ pip), move json.h to
include/openvswitch. This also requires moving lib/{hmap,shash}.h.
Both hmap.h and shash.h were #include-ing "util.h" even though the
headers themselves did not use anything from there, but rather from
include/openvswitch/util.h. Fixing that required including util.h
in several C files mostly due to OVS_NOT_REACHED and things like
xmalloc.
Signed-off-by: Terry Wilson <twilson@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
This allows OVN to take advantage of the client scalability changes
that have been committed to ovsdb-server on master recently.
Conflicts:
Makefile.am
lib/learn.c
The error message should not include bytes already copied from the input
string.
Found by inspection.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>
This saves some cut-and-paste duplicated code elsewhere and will have
additional users in upcoming commits.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Russell Bryant <rbryant@redhat.com>
The following macros are renamed to avoid conflicts with other headers:
* WARN_UNUSED_RESULT to OVS_WARN_UNUSED_RESULT
* PRINTF_FORMAT to OVS_PRINTF_FORMAT
* NO_RETURN to OVS_NO_RETURN
Signed-off-by: Thomas Graf <tgraf@noironetworks.com>
Acked-by: Ben Pfaff <blp@nicira.com>
nodes is allocated through shash_sort() but never freed.
Signed-off-by: Thomas Graf <tgraf@noironetworks.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
json_string_unescape() flagged a backslash at the end of a string as an
error, but of course "\\" is a valid string. This fixes the problem.
VMware-BZ: #1275208
Reported-by: Michael Hu <mhu@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>
This reverts commit 1600fa6853872e16130366351a2c14f6fa8b547c.
Connections that queue up too much data, because they are monitoring a
table that is changing quickly and failing to keep up with the updates,
cause problems with buffer management. Since commit 60533a405b2e
(jsonrpc-server: Disconnect connections that queue too much data.),
ovsdb-server has dealt with them by disconnecting the connection and
letting them start up again with a fresh copy of the database. However,
this is not ideal because of situations where disconnection happens
repeatedly. For example:
- A manager toggles a column back and forth between two or more values
quickly (in which case the data transmitted over the monitoring
connections always increases quickly, without bound).
- A manager repeatedly extends the contents of some column in some row
(in which case the data transmitted over the monitoring connection
grows with O(n**2) in the length of the string).
A better way to deal with this problem is to combine updates when they are
sent to the monitoring connection, if that connection is not keeping up.
In both the above cases, this reduces the data that must be sent to a
manageable amount. An upcoming patch implements this new way. This commit
reverts part of the previous solution that disconnects backlogged
connections, since it is no longer useful.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>
This allows other libraries to use util.h that has already
defined NOT_REACHED.
Signed-off-by: Harold Lim <haroldl@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
where it can't be EOF, cast a value to unsigned char before passing it
to ctype functions to avoid unintended sign extension.
Signed-off-by: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
Signed-off-by: Ben Pfaff <blp@nicira.com>
This is a straight search-and-replace, except that I also removed #include
<assert.h> from each file where there were no assert calls left.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
Casts are sometimes necessary. One common reason that they are necessary
is for discarding a "const" qualifier. However, this can impede
maintenance: if the type of the expression being cast changes, then the
presence of the cast can hide a necessary change in the code that does the
cast. Using CONST_CAST, instead of a bare cast, makes these changes
visible.
Inspired by my own work elsewhere:
http://git.savannah.gnu.org/cgit/pspp.git/tree/src/libpspp/cast.h#n80
Signed-off-by: Ben Pfaff <blp@nicira.com>
Replaced all instances of Nicira Networks(, Inc) to Nicira, Inc.
Feature #10593
Signed-off-by: Raju Subramanian <rsubramanian@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
When json_lex_input() returns false, the parser does not consume the byte
passed in. That byte will get processed again in the next iteration of
the json_parser_feed() loop. Therefore, until now, this code has
double-counted bytes that cause a false return from json_lex_input().
This fixes the problem. Every input byte is now counted only once.
Signed-off-by: Ben Pfaff <blp@nicira.com>
All of these changes avoid using the same name for two local variables
within a same function. None of them are actual bugs as far as I can tell,
but any of them could be confusing to the casual reader.
The one in lib/ovsdb-idl.c is particularly brilliant: inner and outer
loops both using (different) variables named 'i'.
Found with GCC -Wshadow.
RFC 4627 (which defines JSON) says:
The names within an object SHOULD be unique.
In my view, this means that the treatment of duplicate names within a
JSON object is more or less up to the implementation. Until now, the OVS
JSON parser has dealt with duplicates fairly badly: they all get shoved
into the hash table and you get one or the other value semi-randomly
(typically the one added later). This commit makes the behavior
predictable: old values are deleted and replaced by newer values.
Some upcoming code wants to serialize JSON into a "struct ds" dynamic
string buffer, so expose an interface to do this.
This commit doesn't change much, but it renames some functions internal
to json.c to make the naming more consistent.
Also, make jsonrpc_log_msg() use this new function, since it is a more
straightforward way to do what it wants.
The JSON syntax for strings is very reasonable. An upcoming commit will
have a need for a string parser, so make the JSON string parser available
for that.
Also, this change improves the error message for strings that end in the
middle of a \u sequence, so update the tests to match.
The chroots in which we often build Open vSwitch don't have /proc and
thus cannot support /dev/stdin, because on Linux that is a symlink to
/proc/self/fd/0. So avoid using /dev/stdin in the testsuite.
The test for whether a real number was outside the valid range was
imprecise and failed at the edge of the real range. This commit changes
the code to use the C library's strtod(), which presumably does better.
The previous code checked only that JSON objects have members with the
same names. This commit makes json_equal() also check that like-named
members have the same values.
