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

Message ID 1465394115-67693-4-git-send-email-twilson@redhat.com
State Changes Requested
Headers show

Commit Message

Terry Wilson June 8, 2016, 1:55 p.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 |  22 +++++++++++
 python/ovs/json.py | 106 +++++------------------------------------------------
 tests/json.at      |  26 ++++++++++---
 5 files changed, 101 insertions(+), 102 deletions(-)

Comments

Ben Pfaff June 8, 2016, 6:46 p.m. UTC | #1
On Wed, Jun 08, 2016 at 08:55:15AM -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 which is probably because I run "configure" in a
directory different from the source directory:

    (cd python/ && /usr/bin/python3 setup.py build_ext --inplace)
    /bin/bash: line 0: cd: python/: No such file or directory
    Makefile:6149: recipe for target '../python/ovs/_json.cpython-35m-i386-linux-gnu.so' failed

Also it's not necessary to put Makefile commands in parentheses since
they always run in a subshell anyway.

Thanks,

Ben.

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..5b81154 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_py_headers=true])
+      LIBS="$save_LIBS"
+      CPPFLAGS="$save_CPPFLAGS"
+      LDFLAGS="$save_LDFLAGS"
+    fi
+   AM_CONDITIONAL([HAVE_PYTHON3_HEADERS], [test "$have_py_headers" = "true"])])
+  ])
diff --git a/python/automake.mk b/python/automake.mk
index ecad39d..9ae2487 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -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 python/ && rm -f ovs/*.so)
+
+if HAVE_PYTHON_HEADERS
+$(srcdir)/python/ovs/_json.so:
+	(cd 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):
+	(cd 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],