From: Bryan Duxbury Date: Wed, 21 Dec 2011 18:13:29 +0000 (+0000) Subject: THRIFT-1469. java: Java isset space optimization X-Git-Tag: 0.9.1~484 X-Git-Url: https://source.supwisdom.com/gerrit/gitweb?a=commitdiff_plain;h=1e79cb40f5447b514cad823f14fd32235bf6d07b;p=common%2Fthrift.git THRIFT-1469. java: Java isset space optimization This patch gives the generated code some variable-sized options for the isset bit vector. The compiler will attempt to use byte, short, int and long types before reverting to a BitSet for structs with a LOT of optional fields. This should save a fair amount of memory in a lot of cases. Patch: Brian Bloniarz git-svn-id: https://svn.apache.org/repos/asf/thrift/trunk@1221828 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/compiler/cpp/src/generate/t_java_generator.cc b/compiler/cpp/src/generate/t_java_generator.cc index 49f81be4..9f2bbd3d 100644 --- a/compiler/cpp/src/generate/t_java_generator.cc +++ b/compiler/cpp/src/generate/t_java_generator.cc @@ -240,7 +240,8 @@ public: void generate_deep_copy_container(std::ofstream& out, std::string source_name_p1, std::string source_name_p2, std::string result_name, t_type* type); void generate_deep_copy_non_container(std::ofstream& out, std::string source_name, std::string dest_name, t_type* type); - bool has_bit_vector(t_struct* tstruct); + enum isset_type { ISSET_NONE, ISSET_PRIMITIVE, ISSET_BITSET }; + isset_type needs_isset(t_struct* tstruct, std::string *outPrimitiveType = NULL); /** * Helper rendering functions @@ -352,6 +353,7 @@ string t_java_generator::java_type_imports() { "import org.apache.thrift.scheme.StandardScheme;\n\n" + "import org.apache.thrift.scheme.TupleScheme;\n" + "import org.apache.thrift.protocol.TTupleProtocol;\n" + + "import org.apache.thrift.EncodingUtils;\n" + "import java.util.List;\n" + "import java.util.ArrayList;\n" + "import java.util.Map;\n" + @@ -1296,9 +1298,18 @@ void t_java_generator::generate_java_struct_definition(ofstream &out, } } - if (i > 0) { + std::string primitiveType; + switch(needs_isset(tstruct, &primitiveType)) { + case ISSET_NONE: + break; + case ISSET_PRIMITIVE: + indent(out) << "private " << primitiveType << " __isset_bitfield = 0;" << endl; + break; + case ISSET_BITSET: indent(out) << "private BitSet __isset_bit_vector = new BitSet(" << i << ");" << endl; + break; } + if (optionals > 0) { std::string output_string = "private _Fields optionals[] = {"; for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) { @@ -1370,9 +1381,16 @@ void t_java_generator::generate_java_struct_definition(ofstream &out, indent(out) << "public " << tstruct->get_name() << "(" << tstruct->get_name() << " other) {" << endl; indent_up(); - if (has_bit_vector(tstruct)) { + switch(needs_isset(tstruct)) { + case ISSET_NONE: + break; + case ISSET_PRIMITIVE: + indent(out) << "__isset_bitfield = other.__isset_bitfield;" << endl; + break; + case ISSET_BITSET: indent(out) << "__isset_bit_vector.clear();" << endl; indent(out) << "__isset_bit_vector.or(other.__isset_bit_vector);" << endl; + break; } for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) { @@ -1788,6 +1806,7 @@ void t_java_generator::generate_generic_isset_method(std::ofstream& out, t_struc */ void t_java_generator::generate_java_bean_boilerplate(ofstream& out, t_struct* tstruct) { + isset_type issetType = needs_isset(tstruct); const vector& fields = tstruct->get_members(); vector::const_iterator f_iter; for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) { @@ -1928,6 +1947,8 @@ void t_java_generator::generate_java_bean_boilerplate(ofstream& out, indent_up(); if (type_can_be_null(type)) { indent(out) << "this." << field_name << " = null;" << endl; + } else if(issetType == ISSET_PRIMITIVE) { + indent(out) << "__isset_bitfield = EncodingUtils.clearBit(__isset_bitfield, " << isset_field_id(field) << ");" << endl; } else { indent(out) << "__isset_bit_vector.clear(" << isset_field_id(field) << ");" << endl; } @@ -1940,6 +1961,8 @@ void t_java_generator::generate_java_bean_boilerplate(ofstream& out, indent_up(); if (type_can_be_null(type)) { indent(out) << "return this." << field_name << " != null;" << endl; + } else if(issetType == ISSET_PRIMITIVE) { + indent(out) << "return EncodingUtils.testBit(__isset_bitfield, " << isset_field_id(field) << ");" << endl; } else { indent(out) << "return __isset_bit_vector.get(" << isset_field_id(field) << ");" << endl; } @@ -1952,6 +1975,8 @@ void t_java_generator::generate_java_bean_boilerplate(ofstream& out, indent(out) << "if (!value) {" << endl; indent(out) << " this." << field_name << " = null;" << endl; indent(out) << "}" << endl; + } else if(issetType == ISSET_PRIMITIVE) { + indent(out) << "__isset_bitfield = EncodingUtils.setBit(__isset_bitfield, " << isset_field_id(field) << ", value);" << endl; } else { indent(out) << "__isset_bit_vector.set(" << isset_field_id(field) << ", value);" << endl; } @@ -3759,16 +3784,33 @@ void t_java_generator::generate_field_name_constants(ofstream& out, t_struct* ts indent(out) << "}" << endl; } -bool t_java_generator::has_bit_vector(t_struct* tstruct) { +t_java_generator::isset_type t_java_generator::needs_isset(t_struct* tstruct, std::string *outPrimitiveType) { const vector& members = tstruct->get_members(); vector::const_iterator m_iter; + int count = 0; for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) { if (!type_can_be_null(get_true_type((*m_iter)->get_type()))) { - return true; + count++; + } + } + if(count == 0) { + return ISSET_NONE; + } else if(count <= 64) { + if(outPrimitiveType != NULL) { + if(count <= 8) + *outPrimitiveType = "byte"; + else if(count <= 16) + *outPrimitiveType = "short"; + else if(count <= 32) + *outPrimitiveType = "int"; + else if(count <= 64) + *outPrimitiveType = "long"; } + return ISSET_PRIMITIVE; + } else { + return ISSET_BITSET; } - return false; } void t_java_generator::generate_java_struct_clear(std::ofstream& out, t_struct* tstruct) { @@ -3838,9 +3880,19 @@ void t_java_generator::generate_java_struct_write_object(ofstream& out, t_struct void t_java_generator::generate_java_struct_read_object(ofstream& out, t_struct* tstruct) { indent(out) << "private void readObject(java.io.ObjectInputStream in) throws java.io.IOException, ClassNotFoundException {" << endl; indent(out) << " try {" << endl; - if (!tstruct->is_union() && has_bit_vector(tstruct)) { - indent(out) << " // it doesn't seem like you should have to do this, but java serialization is wacky, and doesn't call the default constructor." << endl; - indent(out) << " __isset_bit_vector = new BitSet(1);" << endl; + if (!tstruct->is_union()) { + switch(needs_isset(tstruct)) { + case ISSET_NONE: + break; + case ISSET_PRIMITIVE: + indent(out) << " // it doesn't seem like you should have to do this, but java serialization is wacky, and doesn't call the default constructor." << endl; + indent(out) << " __isset_bitfield = 0;" << endl; + break; + case ISSET_BITSET: + indent(out) << " // it doesn't seem like you should have to do this, but java serialization is wacky, and doesn't call the default constructor." << endl; + indent(out) << " __isset_bit_vector = new BitSet(1);" << endl; + break; + } } indent(out) << " read(new org.apache.thrift.protocol.TCompactProtocol(new org.apache.thrift.transport.TIOStreamTransport(in)));" << endl; indent(out) << " } catch (org.apache.thrift.TException te) {" << endl; diff --git a/lib/java/build.xml b/lib/java/build.xml index c0c2374e..eb741259 100644 --- a/lib/java/build.xml +++ b/lib/java/build.xml @@ -242,6 +242,9 @@ + + + diff --git a/lib/java/src/org/apache/thrift/EncodingUtils.java b/lib/java/src/org/apache/thrift/EncodingUtils.java index 072de93c..bf14ef57 100644 --- a/lib/java/src/org/apache/thrift/EncodingUtils.java +++ b/lib/java/src/org/apache/thrift/EncodingUtils.java @@ -82,4 +82,67 @@ public class EncodingUtils { | ((buf[offset + 2] & 0xff) << 8) | ((buf[offset + 3] & 0xff)); } + /** + * Bitfield utilities. + * Returns true if the bit at position is set in v. + */ + public static final boolean testBit(byte v, int position) { + return testBit((int)v, position); + } + + public static final boolean testBit(short v, int position) { + return testBit((int)v, position); + } + + public static final boolean testBit(int v, int position) { + return (v & (1 << position)) != 0; + } + + public static final boolean testBit(long v, int position) { + return (v & (1L << position)) != 0L; + } + + /** + * Returns v, with the bit at position set to zero. + */ + public static final byte clearBit(byte v, int position) { + return (byte)clearBit((int)v, position); + } + + public static final short clearBit(short v, int position) { + return (short)clearBit((int)v, position); + } + + public static final int clearBit(int v, int position) { + return v & ~(1 << position); + } + + public static final long clearBit(long v, int position) { + return v & ~(1L << position); + } + + /** + * Returns v, with the bit at position set to 1 or 0 depending on value. + */ + public static final byte setBit(byte v, int position, boolean value) { + return (byte)setBit((int)v, position, value); + } + + public static final short setBit(short v, int position, boolean value) { + return (short)setBit((int)v, position, value); + } + + public static final int setBit(int v, int position, boolean value) { + if(value) + return v | (1 << position); + else + return clearBit(v, position); + } + + public static final long setBit(long v, int position, boolean value) { + if(value) + return v | (1L << position); + else + return clearBit(v, position); + } } diff --git a/lib/java/test/org/apache/thrift/TestOptionals.java b/lib/java/test/org/apache/thrift/TestOptionals.java new file mode 100644 index 00000000..d1591ee2 --- /dev/null +++ b/lib/java/test/org/apache/thrift/TestOptionals.java @@ -0,0 +1,88 @@ +/* + * 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. + */ + +package org.apache.thrift; + +import junit.framework.TestCase; + +import thrift.test.Opt4; +import thrift.test.Opt30; +import thrift.test.Opt64; +import thrift.test.Opt80; + +// Exercises the isSet methods using structs from from ManyOptionals.thrift +public class TestOptionals extends TestCase { + public void testEncodingUtils() throws Exception { + assertEquals((short)0x8, EncodingUtils.setBit((short)0, 3, true)); + assertEquals((short)0, EncodingUtils.setBit((short)0x8, 3, false)); + assertEquals(true, EncodingUtils.testBit((short)0x8, 3)); + assertEquals(false, EncodingUtils.testBit((short)0x8, 4)); + + assertEquals((short)Short.MIN_VALUE, EncodingUtils.setBit((short)0, 15, true)); + assertEquals((short)0, EncodingUtils.setBit((short)Short.MIN_VALUE, 15, false)); + assertEquals(true, EncodingUtils.testBit(Short.MIN_VALUE, 15)); + assertEquals(false, EncodingUtils.testBit(Short.MIN_VALUE, 14)); + } + + public void testOpt4() throws Exception { + Opt4 x = new Opt4(); + assertEquals(false, x.isSetDef1()); + x.setDef1(3); + assertEquals(true, x.isSetDef1()); + assertEquals(false, x.isSetDef2()); + + Opt4 copy = new Opt4(x); + assertEquals(true, copy.isSetDef1()); + copy.unsetDef1(); + assertEquals(false, copy.isSetDef1()); + assertEquals(true, x.isSetDef1()); + } + + public void testOpt30() throws Exception { + Opt30 x = new Opt30(); + assertEquals(false, x.isSetDef1()); + x.setDef1(3); + assertEquals(true, x.isSetDef1()); + assertEquals(false, x.isSetDef2()); + } + + public void testOpt64() throws Exception { + Opt64 x = new Opt64(); + assertEquals(false, x.isSetDef1()); + x.setDef1(3); + assertEquals(true, x.isSetDef1()); + assertEquals(false, x.isSetDef2()); + x.setDef64(22); + assertEquals(true, x.isSetDef64()); + assertEquals(false, x.isSetDef63()); + } + + public void testOpt80() throws Exception { + Opt80 x = new Opt80(); + assertEquals(false, x.isSetDef1()); + x.setDef1(3); + assertEquals(true, x.isSetDef1()); + assertEquals(false, x.isSetDef2()); + + Opt80 copy = new Opt80(x); + copy.unsetDef1(); + assertEquals(false, copy.isSetDef1()); + assertEquals(true, x.isSetDef1()); + } +} diff --git a/test/ManyOptionals.thrift b/test/ManyOptionals.thrift new file mode 100644 index 00000000..3fb1d68b --- /dev/null +++ b/test/ManyOptionals.thrift @@ -0,0 +1,231 @@ +/* + * 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. + */ + +// The java codegenerator has a few different codepaths depending +// on how many optionals the struct has; this attempts to exercise +// them. + +namespace java thrift.test + +struct Opt4 { + 1: i32 def1; + 2: i32 def2; + 3: i32 def3; + 4: i32 def4; +} + +struct Opt13 { + 1: i32 def1; + 2: i32 def2; + 3: i32 def3; + 4: i32 def4; + 5: i32 def5; + 6: i32 def6; + 7: i32 def7; + 8: i32 def8; + 9: i32 def9; + 10: i32 def10; + 11: i32 def11; + 12: i32 def12; + 13: i32 def13; +} + +struct Opt30 { + 1: i32 def1; + 2: i32 def2; + 3: i32 def3; + 4: i32 def4; + 5: i32 def5; + 6: i32 def6; + 7: i32 def7; + 8: i32 def8; + 9: i32 def9; + 10: i32 def10; + 11: i32 def11; + 12: i32 def12; + 13: i32 def13; + 14: i32 def14; + 15: i32 def15; + 16: i32 def16; + 17: i32 def17; + 18: i32 def18; + 19: i32 def19; + 20: i32 def20; + 21: i32 def21; + 22: i32 def22; + 23: i32 def23; + 24: i32 def24; + 25: i32 def25; + 26: i32 def26; + 27: i32 def27; + 28: i32 def28; + 29: i32 def29; + 30: i32 def30; +} + +struct Opt64 { + 1: i32 def1; + 2: i32 def2; + 3: i32 def3; + 4: i32 def4; + 5: i32 def5; + 6: i32 def6; + 7: i32 def7; + 8: i32 def8; + 9: i32 def9; + 10: i32 def10; + 11: i32 def11; + 12: i32 def12; + 13: i32 def13; + 14: i32 def14; + 15: i32 def15; + 16: i32 def16; + 17: i32 def17; + 18: i32 def18; + 19: i32 def19; + 20: i32 def20; + 21: i32 def21; + 22: i32 def22; + 23: i32 def23; + 24: i32 def24; + 25: i32 def25; + 26: i32 def26; + 27: i32 def27; + 28: i32 def28; + 29: i32 def29; + 30: i32 def30; + 31: i32 def31; + 32: i32 def32; + 33: i32 def33; + 34: i32 def34; + 35: i32 def35; + 36: i32 def36; + 37: i32 def37; + 38: i32 def38; + 39: i32 def39; + 40: i32 def40; + 41: i32 def41; + 42: i32 def42; + 43: i32 def43; + 44: i32 def44; + 45: i32 def45; + 46: i32 def46; + 47: i32 def47; + 48: i32 def48; + 49: i32 def49; + 50: i32 def50; + 51: i32 def51; + 52: i32 def52; + 53: i32 def53; + 54: i32 def54; + 55: i32 def55; + 56: i32 def56; + 57: i32 def57; + 58: i32 def58; + 59: i32 def59; + 60: i32 def60; + 61: i32 def61; + 62: i32 def62; + 63: i32 def63; + 64: i32 def64; +} + +struct Opt80 { + 1: i32 def1; + 2: i32 def2; + 3: i32 def3; + 4: i32 def4; + 5: i32 def5; + 6: i32 def6; + 7: i32 def7; + 8: i32 def8; + 9: i32 def9; + 10: i32 def10; + 11: i32 def11; + 12: i32 def12; + 13: i32 def13; + 14: i32 def14; + 15: i32 def15; + 16: i32 def16; + 17: i32 def17; + 18: i32 def18; + 19: i32 def19; + 20: i32 def20; + 21: i32 def21; + 22: i32 def22; + 23: i32 def23; + 24: i32 def24; + 25: i32 def25; + 26: i32 def26; + 27: i32 def27; + 28: i32 def28; + 29: i32 def29; + 30: i32 def30; + 31: i32 def31; + 32: i32 def32; + 33: i32 def33; + 34: i32 def34; + 35: i32 def35; + 36: i32 def36; + 37: i32 def37; + 38: i32 def38; + 39: i32 def39; + 40: i32 def40; + 41: i32 def41; + 42: i32 def42; + 43: i32 def43; + 44: i32 def44; + 45: i32 def45; + 46: i32 def46; + 47: i32 def47; + 48: i32 def48; + 49: i32 def49; + 50: i32 def50; + 51: i32 def51; + 52: i32 def52; + 53: i32 def53; + 54: i32 def54; + 55: i32 def55; + 56: i32 def56; + 57: i32 def57; + 58: i32 def58; + 59: i32 def59; + 60: i32 def60; + 61: i32 def61; + 62: i32 def62; + 63: i32 def63; + 64: i32 def64; + 65: i32 def65; + 66: i32 def66; + 67: i32 def67; + 68: i32 def68; + 69: i32 def69; + 70: i32 def70; + 71: i32 def71; + 72: i32 def72; + 73: i32 def73; + 74: i32 def74; + 75: i32 def75; + 76: i32 def76; + 77: i32 def77; + 78: i32 def78; + 79: i32 def79; + 80: i32 def80; +} +