THRIFT-3504 Fix FastbinaryTest.py Client: Python Patch: Nobuaki Sukegawa This closes #757
Project: http://git-wip-us.apache.org/repos/asf/thrift/repo Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/7b894694 Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/7b894694 Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/7b894694 Branch: refs/heads/master Commit: 7b8946941e5bcb1217711115fed3f6c77d73b5ef Parents: 6dde7f1 Author: Nobuaki Sukegawa <[email protected]> Authored: Wed Dec 23 21:45:06 2015 +0900 Committer: Nobuaki Sukegawa <[email protected]> Committed: Sat Jan 2 22:54:16 2016 +0900 ---------------------------------------------------------------------- compiler/cpp/src/generate/t_py_generator.cc | 2 +- lib/py/src/protocol/fastbinary.c | 5 +- lib/py/src/transport/TTransport.py | 7 +- test/DebugProtoTest.thrift | 10 +- test/FastbinaryTest.py | 222 ----------------------- test/Makefile.am | 1 - test/py/FastbinaryTest.py | 215 ++++++++++++++++++++++ test/py/RunClientServer.py | 1 + 8 files changed, 228 insertions(+), 235 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/thrift/blob/7b894694/compiler/cpp/src/generate/t_py_generator.cc ---------------------------------------------------------------------- diff --git a/compiler/cpp/src/generate/t_py_generator.cc b/compiler/cpp/src/generate/t_py_generator.cc index 49c0b57..81e4643 100644 --- a/compiler/cpp/src/generate/t_py_generator.cc +++ b/compiler/cpp/src/generate/t_py_generator.cc @@ -2087,7 +2087,7 @@ void t_py_generator::generate_deserialize_container(ofstream& out, t_type* ttype // Declare variables, read header if (ttype->is_map()) { out << indent() << prefix << " = {}" << endl << indent() << "(" << ktype << ", " << vtype - << ", " << size << " ) = iprot.readMapBegin()" << endl; + << ", " << size << ") = iprot.readMapBegin()" << endl; } else if (ttype->is_set()) { out << indent() << prefix << " = set()" << endl << indent() << "(" << etype << ", " << size << ") = iprot.readSetBegin()" << endl; http://git-wip-us.apache.org/repos/asf/thrift/blob/7b894694/lib/py/src/protocol/fastbinary.c ---------------------------------------------------------------------- diff --git a/lib/py/src/protocol/fastbinary.c b/lib/py/src/protocol/fastbinary.c index a17019b..091a617 100644 --- a/lib/py/src/protocol/fastbinary.c +++ b/lib/py/src/protocol/fastbinary.c @@ -290,7 +290,7 @@ parse_struct_item_spec(StructItemSpec* dest, PyObject* spec_tuple) { // i'd like to use ParseArgs here, but it seems to be a bottleneck. if (PyTuple_Size(spec_tuple) != 5) { - PyErr_Format(PyExc_TypeError, "expecting 5 arguments for spec tuple but got %d", PyTuple_Size(spec_tuple)); + PyErr_Format(PyExc_TypeError, "expecting 5 arguments for spec tuple but got %d", (int)PyTuple_Size(spec_tuple)); return false; } @@ -350,7 +350,7 @@ static void writeDouble(PyObject* outbuf, double dub) { /* --- MAIN RECURSIVE OUTPUT FUNCTION -- */ -static int +static bool output_val(PyObject* output, PyObject* value, TType type, PyObject* typeargs) { /* * Refcounting Strategy: @@ -1185,7 +1185,6 @@ decode_val(DecodeBuffer* input, TType type, PyObject* typeargs) { case T_STRUCT: { StructTypeArgs parsedargs; - PyObject* ret; if (!parse_struct_args(&parsedargs, typeargs)) { return NULL; } http://git-wip-us.apache.org/repos/asf/thrift/blob/7b894694/lib/py/src/transport/TTransport.py ---------------------------------------------------------------------- diff --git a/lib/py/src/transport/TTransport.py b/lib/py/src/transport/TTransport.py index 3fe289a..92dc4cd 100644 --- a/lib/py/src/transport/TTransport.py +++ b/lib/py/src/transport/TTransport.py @@ -139,7 +139,8 @@ class TBufferedTransport(TTransportBase, CReadableTransport): def __init__(self, trans, rbuf_size=DEFAULT_BUFFER): self.__trans = trans self.__wbuf = BufferIO() - self.__rbuf = BufferIO() + # Pass string argument to initialize read buffer as cStringIO.InputType + self.__rbuf = BufferIO(b'') self.__rbuf_size = rbuf_size def isOpen(self): @@ -256,7 +257,7 @@ class TFramedTransport(TTransportBase, CReadableTransport): def __init__(self, trans,): self.__trans = trans - self.__rbuf = BufferIO() + self.__rbuf = BufferIO(b'') self.__wbuf = BufferIO() def isOpen(self): @@ -364,7 +365,7 @@ class TSaslClientTransport(TTransportBase, CReadableTransport): self.sasl = SASLClient(host, service, mechanism, **sasl_kwargs) self.__wbuf = BufferIO() - self.__rbuf = BufferIO() + self.__rbuf = BufferIO(b'') def open(self): if not self.transport.isOpen(): http://git-wip-us.apache.org/repos/asf/thrift/blob/7b894694/test/DebugProtoTest.thrift ---------------------------------------------------------------------- diff --git a/test/DebugProtoTest.thrift b/test/DebugProtoTest.thrift index e7119c4..9726d00 100644 --- a/test/DebugProtoTest.thrift +++ b/test/DebugProtoTest.thrift @@ -62,7 +62,7 @@ struct Nesting { struct HolyMoley { 1: list<OneOfEach> big, - 2: set<list<string>> contain, + 2: set<list<string> (python.immutable = "")> contain, 3: map<string,list<Bonk>> bonks, } @@ -259,10 +259,10 @@ service EmptyService {} // The only purpose of this thing is to increase the size of the generated code // so that ZlibTest has more highly compressible data to play with. struct BlowUp { - 1: map<list<i32>,set<map<i32,string>>> b1; - 2: map<list<i32>,set<map<i32,string>>> b2; - 3: map<list<i32>,set<map<i32,string>>> b3; - 4: map<list<i32>,set<map<i32,string>>> b4; + 1: map<list<i32>(python.immutable = ""),set<map<i32,string> (python.immutable = "")>> b1; + 2: map<list<i32>(python.immutable = ""),set<map<i32,string> (python.immutable = "")>> b2; + 3: map<list<i32>(python.immutable = ""),set<map<i32,string> (python.immutable = "")>> b3; + 4: map<list<i32>(python.immutable = ""),set<map<i32,string> (python.immutable = "")>> b4; } http://git-wip-us.apache.org/repos/asf/thrift/blob/7b894694/test/FastbinaryTest.py ---------------------------------------------------------------------- diff --git a/test/FastbinaryTest.py b/test/FastbinaryTest.py deleted file mode 100755 index 7f6efae..0000000 --- a/test/FastbinaryTest.py +++ /dev/null @@ -1,222 +0,0 @@ -#!/usr/bin/env python - -# -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. -# - -r""" -thrift --gen py DebugProtoTest.thrift -./FastbinaryTest.py -""" - -# TODO(dreiss): Test error cases. Check for memory leaks. - -import sys -sys.path.append('./gen-py') - -import math -from DebugProtoTest import Srv -from DebugProtoTest.ttypes import * -from thrift.transport import TTransport -from thrift.protocol import TBinaryProtocol - -import timeit -from cStringIO import StringIO -from copy import deepcopy -from pprint import pprint - -class TDevNullTransport(TTransport.TTransportBase): - def __init__(self): - pass - def isOpen(self): - return True - -ooe1 = OneOfEach() -ooe1.im_true = True; -ooe1.im_false = False; -ooe1.a_bite = 0xd6; -ooe1.integer16 = 27000; -ooe1.integer32 = 1<<24; -ooe1.integer64 = 6000 * 1000 * 1000; -ooe1.double_precision = math.pi; -ooe1.some_characters = "Debug THIS!"; -ooe1.zomg_unicode = "\xd7\n\a\t"; - -ooe2 = OneOfEach(); -ooe2.integer16 = 16; -ooe2.integer32 = 32; -ooe2.integer64 = 64; -ooe2.double_precision = (math.sqrt(5)+1)/2; -ooe2.some_characters = ":R (me going \"rrrr\")"; -ooe2.zomg_unicode = "\xd3\x80\xe2\x85\xae\xce\x9d\x20"\ - "\xd0\x9d\xce\xbf\xe2\x85\xbf\xd0\xbe"\ - "\xc9\xa1\xd0\xb3\xd0\xb0\xcf\x81\xe2\x84\x8e"\ - "\x20\xce\x91\x74\x74\xce\xb1\xe2\x85\xbd\xce\xba"\ - "\xc7\x83\xe2\x80\xbc"; - -hm = HolyMoley({"big":[], "contain":set(), "bonks":{}}) -hm.big.append(ooe1) -hm.big.append(ooe2) -hm.big[0].a_bite = 0x22; -hm.big[1].a_bite = 0x22; - -hm.contain.add(("and a one", "and a two")) -hm.contain.add(("then a one, two", "three!", "FOUR!")) -hm.contain.add(()) - -hm.bonks["nothing"] = []; -hm.bonks["something"] = [ - Bonk({"type":1, "message":"Wait."}), - Bonk({"type":2, "message":"What?"}), -] -hm.bonks["poe"] = [ - Bonk({"type":3, "message":"quoth"}), - Bonk({"type":4, "message":"the raven"}), - Bonk({"type":5, "message":"nevermore"}), -] - -rs = RandomStuff() -rs.a = 1 -rs.b = 2 -rs.c = 3 -rs.myintlist = range(20) -rs.maps = {1:Wrapper({"foo":Empty()}),2:Wrapper({"foo":Empty()})} -rs.bigint = 124523452435L -rs.triple = 3.14 - -# make sure this splits two buffers in a buffered protocol -rshuge = RandomStuff() -rshuge.myintlist=range(10000) - -my_zero = Srv.Janky_result({"arg":5}) - -def checkWrite(o): - trans_fast = TTransport.TMemoryBuffer() - trans_slow = TTransport.TMemoryBuffer() - prot_fast = TBinaryProtocol.TBinaryProtocolAccelerated(trans_fast) - prot_slow = TBinaryProtocol.TBinaryProtocol(trans_slow) - - o.write(prot_fast) - o.write(prot_slow) - ORIG = trans_slow.getvalue() - MINE = trans_fast.getvalue() - if ORIG != MINE: - print "mine: %s\norig: %s" % (repr(MINE), repr(ORIG)) - -def checkRead(o): - prot = TBinaryProtocol.TBinaryProtocol(TTransport.TMemoryBuffer()) - o.write(prot) - - slow_version_binary = prot.trans.getvalue() - - prot = TBinaryProtocol.TBinaryProtocolAccelerated( - TTransport.TMemoryBuffer(slow_version_binary)) - c = o.__class__() - c.read(prot) - if c != o: - print "copy: " - pprint(eval(repr(c))) - print "orig: " - pprint(eval(repr(o))) - - prot = TBinaryProtocol.TBinaryProtocolAccelerated( - TTransport.TBufferedTransport( - TTransport.TMemoryBuffer(slow_version_binary))) - c = o.__class__() - c.read(prot) - if c != o: - print "copy: " - pprint(eval(repr(c))) - print "orig: " - pprint(eval(repr(o))) - - -def doTest(): - checkWrite(hm) - no_set = deepcopy(hm) - no_set.contain = set() - checkRead(no_set) - checkWrite(rs) - checkRead(rs) - checkWrite(rshuge) - checkRead(rshuge) - checkWrite(my_zero) - checkRead(my_zero) - checkRead(Backwards({"first_tag2":4, "second_tag1":2})) - - # One case where the serialized form changes, but only superficially. - o = Backwards({"first_tag2":4, "second_tag1":2}) - trans_fast = TTransport.TMemoryBuffer() - trans_slow = TTransport.TMemoryBuffer() - prot_fast = TBinaryProtocol.TBinaryProtocolAccelerated(trans_fast) - prot_slow = TBinaryProtocol.TBinaryProtocol(trans_slow) - - o.write(prot_fast) - o.write(prot_slow) - ORIG = trans_slow.getvalue() - MINE = trans_fast.getvalue() - if ORIG == MINE: - print "That shouldn't happen." - - - prot = TBinaryProtocol.TBinaryProtocolAccelerated(TTransport.TMemoryBuffer()) - o.write(prot) - prot = TBinaryProtocol.TBinaryProtocol( - TTransport.TMemoryBuffer( - prot.trans.getvalue())) - c = o.__class__() - c.read(prot) - if c != o: - print "copy: " - pprint(eval(repr(c))) - print "orig: " - pprint(eval(repr(o))) - - - -def doBenchmark(): - - iters = 25000 - - setup = """ -from __main__ import hm, rs, TDevNullTransport -from thrift.protocol import TBinaryProtocol -trans = TDevNullTransport() -prot = TBinaryProtocol.TBinaryProtocol%s(trans) -""" - - setup_fast = setup % "Accelerated" - setup_slow = setup % "" - - print "Starting Benchmarks" - - print "HolyMoley Standard = %f" % \ - timeit.Timer('hm.write(prot)', setup_slow).timeit(number=iters) - print "HolyMoley Acceler. = %f" % \ - timeit.Timer('hm.write(prot)', setup_fast).timeit(number=iters) - - print "FastStruct Standard = %f" % \ - timeit.Timer('rs.write(prot)', setup_slow).timeit(number=iters) - print "FastStruct Acceler. = %f" % \ - timeit.Timer('rs.write(prot)', setup_fast).timeit(number=iters) - - - -doTest() -doBenchmark() - http://git-wip-us.apache.org/repos/asf/thrift/blob/7b894694/test/Makefile.am ---------------------------------------------------------------------- diff --git a/test/Makefile.am b/test/Makefile.am index 593b1c4..c1fd589 100755 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -124,7 +124,6 @@ EXTRA_DIST = \ tests.json \ ThriftTest.thrift \ TypedefTest.thrift \ - FastbinaryTest.py \ result.html \ README.md \ valgrind.suppress http://git-wip-us.apache.org/repos/asf/thrift/blob/7b894694/test/py/FastbinaryTest.py ---------------------------------------------------------------------- diff --git a/test/py/FastbinaryTest.py b/test/py/FastbinaryTest.py new file mode 100755 index 0000000..0583373 --- /dev/null +++ b/test/py/FastbinaryTest.py @@ -0,0 +1,215 @@ +#!/usr/bin/env python + +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# + +r""" +PYTHONPATH=./gen-py:../../lib/py/build/lib... ./FastbinaryTest.py +""" + +# TODO(dreiss): Test error cases. Check for memory leaks. + +import math +import timeit + +from copy import deepcopy +from pprint import pprint + +from thrift.transport import TTransport +from thrift.protocol import TBinaryProtocol + +from DebugProtoTest import Srv +from DebugProtoTest.ttypes import Backwards, Bonk, Empty, HolyMoley, OneOfEach, RandomStuff, Wrapper + + +class TDevNullTransport(TTransport.TTransportBase): + def __init__(self): + pass + + def isOpen(self): + return True + +ooe1 = OneOfEach() +ooe1.im_true = True +ooe1.im_false = False +ooe1.a_bite = 0xd6 +ooe1.integer16 = 27000 +ooe1.integer32 = 1 << 24 +ooe1.integer64 = 6000 * 1000 * 1000 +ooe1.double_precision = math.pi +ooe1.some_characters = "Debug THIS!" +ooe1.zomg_unicode = "\xd7\n\a\t" + +ooe2 = OneOfEach() +ooe2.integer16 = 16 +ooe2.integer32 = 32 +ooe2.integer64 = 64 +ooe2.double_precision = (math.sqrt(5) + 1) / 2 +ooe2.some_characters = ":R (me going \"rrrr\")" +ooe2.zomg_unicode = "\xd3\x80\xe2\x85\xae\xce\x9d\x20"\ + "\xd0\x9d\xce\xbf\xe2\x85\xbf\xd0\xbe"\ + "\xc9\xa1\xd0\xb3\xd0\xb0\xcf\x81\xe2\x84\x8e"\ + "\x20\xce\x91\x74\x74\xce\xb1\xe2\x85\xbd\xce\xba"\ + "\xc7\x83\xe2\x80\xbc" + +hm = HolyMoley(**{"big": [], "contain": set(), "bonks": {}}) +hm.big.append(ooe1) +hm.big.append(ooe2) +hm.big[0].a_bite = 0x22 +hm.big[1].a_bite = 0x22 + +hm.contain.add(("and a one", "and a two")) +hm.contain.add(("then a one, two", "three!", "FOUR!")) +hm.contain.add(()) + +hm.bonks["nothing"] = [] +hm.bonks["something"] = [ + Bonk(**{"type": 1, "message": "Wait."}), + Bonk(**{"type": 2, "message": "What?"}), +] +hm.bonks["poe"] = [ + Bonk(**{"type": 3, "message": "quoth"}), + Bonk(**{"type": 4, "message": "the raven"}), + Bonk(**{"type": 5, "message": "nevermore"}), +] + +rs = RandomStuff() +rs.a = 1 +rs.b = 2 +rs.c = 3 +rs.myintlist = range(20) +rs.maps = {1: Wrapper(**{"foo": Empty()}), 2: Wrapper(**{"foo": Empty()})} +rs.bigint = 124523452435 +rs.triple = 3.14 + +# make sure this splits two buffers in a buffered protocol +rshuge = RandomStuff() +rshuge.myintlist = range(10000) + +my_zero = Srv.Janky_result(**{"success": 5}) + + +def check_write(o): + trans_fast = TTransport.TMemoryBuffer() + trans_slow = TTransport.TMemoryBuffer() + prot_fast = TBinaryProtocol.TBinaryProtocolAccelerated(trans_fast) + prot_slow = TBinaryProtocol.TBinaryProtocol(trans_slow) + + o.write(prot_fast) + o.write(prot_slow) + ORIG = trans_slow.getvalue() + MINE = trans_fast.getvalue() + if ORIG != MINE: + print("mine: %s\norig: %s" % (repr(MINE), repr(ORIG))) + + +def check_read(o): + prot = TBinaryProtocol.TBinaryProtocol(TTransport.TMemoryBuffer()) + o.write(prot) + + slow_version_binary = prot.trans.getvalue() + + prot = TBinaryProtocol.TBinaryProtocolAccelerated( + TTransport.TMemoryBuffer(slow_version_binary)) + c = o.__class__() + c.read(prot) + if c != o: + print("copy: ") + pprint(eval(repr(c))) + print("orig: ") + pprint(eval(repr(o))) + + prot = TBinaryProtocol.TBinaryProtocolAccelerated( + TTransport.TBufferedTransport( + TTransport.TMemoryBuffer(slow_version_binary))) + c = o.__class__() + c.read(prot) + if c != o: + print("copy: ") + pprint(eval(repr(c))) + print("orig: ") + pprint(eval(repr(o))) + + +def do_test(): + check_write(hm) + check_read(HolyMoley()) + no_set = deepcopy(hm) + no_set.contain = set() + check_read(no_set) + check_write(rs) + check_read(rs) + check_write(rshuge) + check_read(rshuge) + check_write(my_zero) + check_read(my_zero) + check_read(Backwards(**{"first_tag2": 4, "second_tag1": 2})) + + # One case where the serialized form changes, but only superficially. + o = Backwards(**{"first_tag2": 4, "second_tag1": 2}) + trans_fast = TTransport.TMemoryBuffer() + trans_slow = TTransport.TMemoryBuffer() + prot_fast = TBinaryProtocol.TBinaryProtocolAccelerated(trans_fast) + prot_slow = TBinaryProtocol.TBinaryProtocol(trans_slow) + + o.write(prot_fast) + o.write(prot_slow) + ORIG = trans_slow.getvalue() + MINE = trans_fast.getvalue() + assert id(ORIG) != id(MINE) + + prot = TBinaryProtocol.TBinaryProtocolAccelerated(TTransport.TMemoryBuffer()) + o.write(prot) + prot = TBinaryProtocol.TBinaryProtocol( + TTransport.TMemoryBuffer(prot.trans.getvalue())) + c = o.__class__() + c.read(prot) + if c != o: + print("copy: ") + pprint(eval(repr(c))) + print("orig: ") + pprint(eval(repr(o))) + + +def do_benchmark(iters=5000): + setup = """ +from __main__ import hm, rs, TDevNullTransport +from thrift.protocol import TBinaryProtocol +trans = TDevNullTransport() +prot = TBinaryProtocol.TBinaryProtocol%s(trans) +""" + + setup_fast = setup % "Accelerated" + setup_slow = setup % "" + + print("Starting Benchmarks") + + print("HolyMoley Standard = %f" % + timeit.Timer('hm.write(prot)', setup_slow).timeit(number=iters)) + print("HolyMoley Acceler. = %f" % + timeit.Timer('hm.write(prot)', setup_fast).timeit(number=iters)) + + print("FastStruct Standard = %f" % + timeit.Timer('rs.write(prot)', setup_slow).timeit(number=iters)) + print("FastStruct Acceler. = %f" % + timeit.Timer('rs.write(prot)', setup_fast).timeit(number=iters)) + +if __name__ == '__main__': + do_test() + do_benchmark() http://git-wip-us.apache.org/repos/asf/thrift/blob/7b894694/test/py/RunClientServer.py ---------------------------------------------------------------------- diff --git a/test/py/RunClientServer.py b/test/py/RunClientServer.py index f084a41..f83f557 100755 --- a/test/py/RunClientServer.py +++ b/test/py/RunClientServer.py @@ -37,6 +37,7 @@ DEFAULT_LIBDIR_GLOB = os.path.join(ROOT_DIR, 'lib', 'py', 'build', 'lib.*') DEFAULT_LIBDIR_PY3 = os.path.join(ROOT_DIR, 'lib', 'py', 'build', 'lib') SCRIPTS = [ + 'FastbinaryTest.py', 'TestFrozen.py', 'TSimpleJSONProtocolTest.py', 'SerializationTest.py',
