[ovs-dev,v5] JSON serialization via Python's json lib
diff mbox

Message ID 1465541682-45828-1-git-send-email-twilson@redhat.com
State Changes Requested
Headers show

Commit Message

Terry Wilson June 10, 2016, 6:54 a.m. UTC
There is no particularly good reason to use our own Python JSON
serialization implementation when serialization can be done faster
with Python's built-in JSON library.

A few tests were changed due to Python's default JSON library
returning slightly more precise floating point numbers.

Signed-off-by: Terry Wilson <twilson@redhat.com>
---
 configure.ac       |   2 +
 m4/openvswitch.m4  |  47 ++++++++++++++++++++++++
 python/automake.mk |  26 ++++++++++++-
 python/ovs/json.py | 106 +++++------------------------------------------------
 tests/json.at      |  26 ++++++++++---
 5 files changed, 103 insertions(+), 104 deletions(-)

Comments

Ben Pfaff June 23, 2016, 11:15 p.m. UTC | #1
On Fri, Jun 10, 2016 at 01:54:42AM -0500, Terry Wilson wrote:
> There is no particularly good reason to use our own Python JSON
> serialization implementation when serialization can be done faster
> with Python's built-in JSON library.
> 
> A few tests were changed due to Python's default JSON library
> returning slightly more precise floating point numbers.
> 
> Signed-off-by: Terry Wilson <twilson@redhat.com>

I get a build failure when builddir != srcdir:

-*- mode: compilation; default-directory: "~/nicira/ovs/" -*-
Compilation started at Thu Jun 23 16:15:28

ovs-build-all _build
make  all-recursive
Making all in datapath
make[3]: 'distfiles' is up to date.
make[2]: Entering directory '/home/blp/nicira/ovs/_build'
cd ../python/ && /usr/bin/python3 setup.py build_ext --inplace
running build_ext
building 'ovs._json' extension
i586-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -g -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC -I/usr/include/python3.5m -c ovs/_json.c -o build/temp.linux-x86_64-3.5/ovs/_json.o
ovs/_json.c:2:34: fatal error: openvswitch/lib/json.h: No such file or directory
 #include <openvswitch/lib/json.h>
                                  ^
compilation terminated.
***************************************************************************
WARNING: The C extension could not be compiled, speedups are not enabled.
Failure information, if any, is above.
Retrying the build without the C extension.
***************************************************************************
running build_ext
make[2]: Leaving directory '/home/blp/nicira/ovs/_build'

Compilation finished at Thu Jun 23 16:15:29

Patch
diff mbox

diff --git a/configure.ac b/configure.ac
index 05d80d5..5472a52 100644
--- a/configure.ac
+++ b/configure.ac
@@ -96,6 +96,8 @@  OVS_CHECK_LIBCAPNG
 OVS_CHECK_LOGDIR
 OVS_CHECK_PYTHON
 OVS_CHECK_PYTHON3
+OVS_CHECK_PYTHON_HEADERS
+OVS_CHECK_PYTHON3_HEADERS
 OVS_CHECK_FLAKE8
 OVS_CHECK_DOT
 OVS_CHECK_IF_PACKET
diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4
index a448223..60ae114 100644
--- a/m4/openvswitch.m4
+++ b/m4/openvswitch.m4
@@ -589,3 +589,50 @@  AC_DEFUN([OVS_CHECK_PRAGMA_MESSAGE],
      [AC_DEFINE(HAVE_PRAGMA_MESSAGE,1,[Define if compiler supports #pragma
      message directive])])
   ])
+
+dnl OVS_CHECK_PYTHON_HEADERS
+AC_DEFUN([OVS_CHECK_PYTHON_HEADERS],
+  [AC_REQUIRE([OVS_CHECK_PYTHON])
+    AC_PATH_PROG([PYTHON_CONFIG], python-config, no)
+    if test "$PYTHON_CONFIG" != no; then
+      PYTHON_INCLUDES=`$PYTHON_CONFIG --includes`
+      PYTHON_LIBS=`$PYTHON_CONFIG --libs`
+      PYTHON_LDFLAGS=`$PYTHON_CONFIG --ldflags`
+      save_LIBS="$LIBS"
+      save_CPPFLAGS="$CPPFLAGS"
+      save_LDFLAGS="$LDFLAGS"
+      LIBS="$PYTHON_LIBS"
+      LDFLAGS="$PYTHON_LDFLAGS"
+      CPPFLAGS="$PYTHON_INCLUDES"
+      AC_LINK_IFELSE(
+        [AC_LANG_PROGRAM([#include <Python.h>],[])],
+                         [have_py_headers=true])
+      LIBS="$save_LIBS"
+      CPPFLAGS="$save_CPPFLAGS"
+      LDFLAGS="$save_LDFLAGS"
+    fi
+   AM_CONDITIONAL([HAVE_PYTHON_HEADERS], [test "$have_py_headers" = "true"])])
+  ])
+
+AC_DEFUN([OVS_CHECK_PYTHON3_HEADERS],
+  [AC_REQUIRE([OVS_CHECK_PYTHON3])
+    AC_PATH_PROG([PYTHON3_CONFIG], python3-config, no)
+    if test "$PYTHON3_CONFIG" != no; then
+      PYTHON3_INCLUDES=`$PYTHON3_CONFIG --includes`
+      PYTHON3_LIBS=`$PYTHON3_CONFIG --libs`
+      PYTHON3_LDFLAGS=`$PYTHON3_CONFIG --ldflags`
+      save_LIBS="$LIBS"
+      save_CPPFLAGS="$CPPFLAGS"
+      save_LDFLAGS="$LDFLAGS"
+      LIBS="$PYTHON3_LIBS"
+      LDFLAGS="$PYTHON3_LDFLAGS"
+      CPPFLAGS="$PYTHON3_INCLUDES"
+      AC_LINK_IFELSE(
+        [AC_LANG_PROGRAM([#include <Python.h>],[])],
+                         [have_py3_headers=true])
+      LIBS="$save_LIBS"
+      CPPFLAGS="$save_CPPFLAGS"
+      LDFLAGS="$save_LDFLAGS"
+    fi
+   AM_CONDITIONAL([HAVE_PYTHON3_HEADERS], [test "$have_py3_headers" = "true"])])
+  ])
diff --git a/python/automake.mk b/python/automake.mk
index ecad39d..b7e5254 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -79,10 +79,10 @@  ovs-install-data-local:
 	rm python/ovs/dirs.py.tmp
 
 python-sdist: $(srcdir)/python/ovs/version.py $(ovs_pyfiles) python/ovs/dirs.py
-	(cd python/ && $(PYTHON) setup.py sdist)
+	cd $(srcdir)/python/ && $(PYTHON) setup.py sdist
 
 pypi-upload: $(srcdir)/python/ovs/version.py $(ovs_pyfiles) python/ovs/dirs.py
-	(cd python/ && $(PYTHON) setup.py sdist upload)
+	cd $(srcdir)/python/ && $(PYTHON) setup.py sdist upload
 else
 ovs-install-data-local:
 	@:
@@ -112,3 +112,25 @@  $(srcdir)/python/ovs/dirs.py: python/ovs/dirs.py.template
 		< $? > $@.tmp && \
 	mv $@.tmp $@
 EXTRA_DIST += python/ovs/dirs.py.template
+
+.PHONY : clean_python_extensions
+clean_python_extensions:
+	cd $(srcdir)/python/ && rm -f ovs/*.so
+
+if HAVE_PYTHON_HEADERS
+$(srcdir)/python/ovs/_json.so: lib/libopenvswitch.la
+	cd $(srcdir)/python/ && $(PYTHON) setup.py build_ext --inplace
+
+ALL_LOCAL += $(srcdir)/python/ovs/_json.so
+endif
+
+if HAVE_PYTHON3_HEADERS
+PY3_EXT_NAME=$(srcdir)/python/ovs/_json$(shell $(PYTHON3) -c \
+			 "from distutils import sysconfig;print(sysconfig.get_config_var('EXT_SUFFIX'))")
+$(PY3_EXT_NAME): lib/libopenvswitch.la
+	cd $(srcdir)/python/ && $(PYTHON3) setup.py build_ext --inplace
+
+ALL_LOCAL += $(PY3_EXT_NAME)
+endif
+
+CLEAN_LOCAL += clean_python_extensions
diff --git a/python/ovs/json.py b/python/ovs/json.py
index ea0400a..ddf5dd2 100644
--- a/python/ovs/json.py
+++ b/python/ovs/json.py
@@ -12,11 +12,13 @@ 
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from __future__ import absolute_import
+import functools
+import json
 import re
 import sys
 
 import six
-from six.moves import range
 
 try:
     import ovs._json
@@ -25,112 +27,24 @@  except ImportError:
 
 __pychecker__ = 'no-stringiter'
 
-escapes = {ord('"'): u"\\\"",
-           ord("\\"): u"\\\\",
-           ord("\b"): u"\\b",
-           ord("\f"): u"\\f",
-           ord("\n"): u"\\n",
-           ord("\r"): u"\\r",
-           ord("\t"): u"\\t"}
-for esc in range(32):
-    if esc not in escapes:
-        escapes[esc] = u"\\u%04x" % esc
-
 SPACES_PER_LEVEL = 2
-
-
-class _Serializer(object):
-    def __init__(self, stream, pretty, sort_keys):
-        self.stream = stream
-        self.pretty = pretty
-        self.sort_keys = sort_keys
-        self.depth = 0
-
-    def __serialize_string(self, s):
-        self.stream.write(u'"%s"' % ''.join(escapes.get(ord(c), c) for c in s))
-
-    def __indent_line(self):
-        if self.pretty:
-            self.stream.write('\n')
-            self.stream.write(' ' * (SPACES_PER_LEVEL * self.depth))
-
-    def serialize(self, obj):
-        if obj is None:
-            self.stream.write(u"null")
-        elif obj is False:
-            self.stream.write(u"false")
-        elif obj is True:
-            self.stream.write(u"true")
-        elif isinstance(obj, six.integer_types):
-            self.stream.write(u"%d" % obj)
-        elif isinstance(obj, float):
-            self.stream.write("%.15g" % obj)
-        elif isinstance(obj, six.text_type):
-            # unicode() on Python 2, or str() in Python 3 (always unicode)
-            self.__serialize_string(obj)
-        elif isinstance(obj, str):
-            # This is for Python 2, where this comes out to unicode(str()).
-            # For Python 3, it's str(str()), but it's harmless.
-            self.__serialize_string(six.text_type(obj))
-        elif isinstance(obj, dict):
-            self.stream.write(u"{")
-
-            self.depth += 1
-            self.__indent_line()
-
-            if self.sort_keys:
-                items = sorted(obj.items())
-            else:
-                items = six.iteritems(obj)
-            for i, (key, value) in enumerate(items):
-                if i > 0:
-                    self.stream.write(u",")
-                    self.__indent_line()
-                self.__serialize_string(six.text_type(key))
-                self.stream.write(u":")
-                if self.pretty:
-                    self.stream.write(u' ')
-                self.serialize(value)
-
-            self.stream.write(u"}")
-            self.depth -= 1
-        elif isinstance(obj, (list, tuple)):
-            self.stream.write(u"[")
-            self.depth += 1
-
-            if obj:
-                self.__indent_line()
-
-                for i, value in enumerate(obj):
-                    if i > 0:
-                        self.stream.write(u",")
-                        self.__indent_line()
-                    self.serialize(value)
-
-            self.depth -= 1
-            self.stream.write(u"]")
-        else:
-            raise Exception("can't serialize %s as JSON" % obj)
+dumper = functools.partial(json.dumps, separators=(",", ":"),
+                           ensure_ascii=False)
 
 
 def to_stream(obj, stream, pretty=False, sort_keys=True):
-    _Serializer(stream, pretty, sort_keys).serialize(obj)
+    stream.write(dumper(obj, indent=SPACES_PER_LEVEL if pretty else None,
+                        sort_keys=sort_keys))
 
 
 def to_file(obj, name, pretty=False, sort_keys=True):
-    stream = open(name, "w")
-    try:
+    with open(name, "w") as stream:
         to_stream(obj, stream, pretty, sort_keys)
-    finally:
-        stream.close()
 
 
 def to_string(obj, pretty=False, sort_keys=True):
-    output = six.StringIO()
-    to_stream(obj, output, pretty, sort_keys)
-    s = output.getvalue()
-    output.close()
-    return s
+    return dumper(obj, indent=SPACES_PER_LEVEL if pretty else None,
+                  sort_keys=sort_keys)
 
 
 def from_stream(stream):
diff --git a/tests/json.at b/tests/json.at
index 32d7fff..ba7d4bb 100644
--- a/tests/json.at
+++ b/tests/json.at
@@ -1,4 +1,4 @@ 
-m4_define([JSON_CHECK_POSITIVE_C], 
+m4_define([JSON_CHECK_POSITIVE_C],
   [AT_SETUP([$1])
    AT_KEYWORDS([json positive])
    AT_CHECK([printf %s "AS_ESCAPE([$2])" > input])
@@ -11,7 +11,7 @@  m4_define([JSON_CHECK_POSITIVE_C],
 # JSON_CHECK_POSITIVE_PY(TITLE, INPUT, OUTPUT, TEST-JSON-ARGS,
 #                        PYTHON-CHCEK, PYTHON-BIN)
 #
-m4_define([JSON_CHECK_POSITIVE_PY], 
+m4_define([JSON_CHECK_POSITIVE_PY],
   [AT_SETUP([$1])
    AT_KEYWORDS([json positive Python])
    AT_SKIP_IF([test $5 = no])
@@ -41,6 +41,12 @@  m4_define([JSON_CHECK_POSITIVE],
    JSON_CHECK_POSITIVE_PY([$1 - Python3], [$2], [$3], [$4],
                           [$HAVE_PYTHON3], [$PYTHON3])])
 
+m4_define([JSON_CHECK_POSITIVE_PY23],
+  [JSON_CHECK_POSITIVE_PY([$1 - Python2], [$2], [$3], [$4],
+                          [$HAVE_PYTHON], [$PYTHON])
+   JSON_CHECK_POSITIVE_PY([$1 - Python3], [$2], [$3], [$4],
+                          [$HAVE_PYTHON3], [$PYTHON3])])
+
 m4_define([JSON_CHECK_NEGATIVE_C],
   [AT_SETUP([$1])
    AT_KEYWORDS([json negative])
@@ -216,10 +222,14 @@  JSON_CHECK_POSITIVE(
 # It seems likely that the following test will fail on some system that
 # rounds slightly differently in arithmetic or in printf, but I'd like
 # to keep it this way until we run into such a system.
-JSON_CHECK_POSITIVE(
-  [large integers that overflow to reals], 
+JSON_CHECK_POSITIVE_C(
+  [C - large integers that overflow to reals],
   [[[9223372036854775807000, -92233720368547758080000]]],
   [[[9.22337203685478e+21,-9.22337203685478e+22]]])
+JSON_CHECK_POSITIVE_PY23(
+  [large integers that overflow to reals],
+  [[[9223372036854775807000, -92233720368547758080000]]],
+  [[[9.223372036854776e+21,-9.223372036854776e+22]]])
 
 JSON_CHECK_POSITIVE(
   [negative zero],
@@ -237,10 +247,14 @@  JSON_CHECK_POSITIVE(
 # It seems likely that the following test will fail on some system that
 # rounds slightly differently in arithmetic or in printf, but I'd like
 # to keep it this way until we run into such a system.
-JSON_CHECK_POSITIVE(
-  [+/- DBL_MAX],
+JSON_CHECK_POSITIVE_C(
+  [C - +/- DBL_MAX],
   [[[1.7976931348623157e+308, -1.7976931348623157e+308]]],
   [[[1.79769313486232e+308,-1.79769313486232e+308]]])
+JSON_CHECK_POSITIVE_PY23(
+  [+/- DBL_MAX],
+  [[[1.7976931348623157e+308, -1.7976931348623157e+308]]],
+  [[[1.7976931348623157e+308,-1.7976931348623157e+308]]])
 
 JSON_CHECK_POSITIVE(
   [negative reals],