Thrift PHP generation Redux

Summary: Chopping the amount of code generated by Thrift for PHP services by two orders of magnitude (approx 25% of the previous size). This is done via putting more logic in a dynamic base class and taking it out of the generated code. Hopefully this wins back the CPU cycles paid just to load code from APC at the cost of a marginal increase in dynamic execution runtime.

Reviewed By: sgrimm, dreiss

Test Plan: Ran all the tests in trunk/test/php, also tested the API generate code and Falcon, etc. in my sandbox


git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@665328 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/compiler/cpp/src/generate/t_php_generator.cc b/compiler/cpp/src/generate/t_php_generator.cc
index 2fb4e18..848e917 100644
--- a/compiler/cpp/src/generate/t_php_generator.cc
+++ b/compiler/cpp/src/generate/t_php_generator.cc
@@ -35,8 +35,9 @@
   // Include other Thrift includes
   const vector<t_program*>& includes = program_->get_includes();
   for (size_t i = 0; i < includes.size(); ++i) {
+    string package = includes[i]->get_name();
     f_types_ <<
-      "include_once $GLOBALS['THRIFT_ROOT'].'/packages/" + includes[i]->get_name() << "_types.php';" << endl;
+      "include_once $GLOBALS['THRIFT_ROOT'].'/packages/" << package << "/" << package << "_types.php';" << endl;
   }
   f_types_ << endl;
 
@@ -283,6 +284,76 @@
   generate_php_struct_definition(f_types_, tstruct, is_exception);
 }
 
+void t_php_generator::generate_php_type_spec(ofstream& out,
+                                             t_type* t) {
+  t = get_true_type(t);
+  indent(out) << "'type' => " << type_to_enum(t) << "," << endl;
+
+  if (t->is_base_type() || t->is_enum()) {
+    // Noop, type is all we need
+  } else if (t->is_struct() || t->is_xception()) {
+    indent(out) << "'class' => '" << t->get_name() <<"'," << endl;
+  } else if (t->is_map()) {
+    t_type* ktype = get_true_type(((t_map*)t)->get_key_type());
+    t_type* vtype = get_true_type(((t_map*)t)->get_val_type());
+    indent(out) << "'ktype' => " << type_to_enum(ktype) << "," << endl;
+    indent(out) << "'vtype' => " << type_to_enum(vtype) << "," << endl;
+    indent(out) << "'key' => array(" << endl;
+    indent_up();
+    generate_php_type_spec(out, ktype);
+    indent_down();
+    indent(out) << ")," << endl;
+    indent(out) << "'val' => array(" << endl;
+    indent_up();
+    generate_php_type_spec(out, vtype);
+    indent(out) << ")," << endl;
+    indent_down();
+  } else if (t->is_list() || t->is_set()) {
+    t_type* etype;
+    if (t->is_list()) {
+      etype = get_true_type(((t_list*)t)->get_elem_type());
+    } else {
+      etype = get_true_type(((t_set*)t)->get_elem_type());
+    }
+    indent(out) << "'etype' => " << type_to_enum(etype) <<"," << endl;
+    indent(out) << "'elem' => array(" << endl;
+    indent_up();
+    generate_php_type_spec(out, etype);
+    indent(out) << ")," << endl;
+    indent_down();
+  } else {
+    throw "compiler error: no type for php struct spec field";
+  }
+
+}
+
+/**
+ * Generates the struct specification structure, which fully qualifies enough
+ * type information to generalize serialization routines.
+ */
+void t_php_generator::generate_php_struct_spec(ofstream& out,
+                                               t_struct* tstruct) {
+  indent(out) << "static $_TSPEC = array(" << endl;
+  indent_up();
+
+  const vector<t_field*>& members = tstruct->get_members();
+  vector<t_field*>::const_iterator m_iter;
+  for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
+    t_type* t = get_true_type((*m_iter)->get_type());
+    indent(out) << (*m_iter)->get_key() << " => array(" << endl;
+    indent_up();
+    out <<
+      indent() << "'var' => '" << (*m_iter)->get_name() << "'," << endl;
+    generate_php_type_spec(out, t);
+    indent(out) << ")," << endl;
+    indent_down();
+  }
+
+  indent_down();
+  indent(out) << "  );" << endl;
+}
+
+
 /**
  * Generates a struct definition for a thrift data type. This is nothing in PHP
  * where the objects are all just associative arrays (unless of course we
@@ -299,12 +370,16 @@
   out <<
     "class " << php_namespace(tstruct->get_program()) << tstruct->get_name();
   if (is_exception) {
-    out << " extends Exception";
+    out << " extends TException";
+  } else {
+    out << " extends TBase";
   }
   out <<
     " {" << endl;
   indent_up();
 
+  generate_php_struct_spec(out, tstruct);
+
   for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
     string dval = "null";
     t_type* t = get_true_type((*m_iter)->get_type());
@@ -331,21 +406,12 @@
     }
 
     out <<
-      indent() << "if (is_array($vals)) {" << endl;
-    indent_up();
-    for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
-      out <<
-        indent() << "if (isset($vals['" << (*m_iter)->get_name() << "'])) {" << endl <<
-        indent() << "  $this->" << (*m_iter)->get_name() << " = $vals['" << (*m_iter)->get_name() << "'];" << endl <<
-        indent() << "}" << endl;
-    }
-    indent_down();
-    out <<
+      indent() << "if (is_array($vals)) {" << endl <<
+      indent() << "  parent::construct(self::$_TSPEC, $vals);" << endl <<
       indent() << "}" << endl;
-    indent_down();
-    out <<
-      indent() << "}" << endl <<
-      endl;
+    scope_down(out);
+
+    out << endl;
   }
 
   out <<
@@ -372,9 +438,14 @@
   vector<t_field*>::const_iterator f_iter;
 
   indent(out) <<
-    "public function read($input) " << endl;
+    "public function read($input)" << endl;
   scope_up(out);
 
+  // TODO(mcslee): Testing this new jonx!
+  indent(out) << "return $this->_read('" << tstruct->get_name() << "', self::$_TSPEC, $input);" << endl;
+  scope_down(out);
+  return;
+
   out <<
     indent() << "$xfer = 0;" << endl <<
     indent() << "$fname = null;" << endl <<
@@ -496,6 +567,11 @@
   }
   indent_up();
 
+  // TODO(mcslee): Testing this new j0nx
+  indent(out) << "return $this->_write('" << tstruct->get_name() << "', self::$_TSPEC, $output);" << endl;
+  scope_down(out);
+  return;
+
   indent(out) <<
     "$xfer = 0;" << endl;
 
@@ -586,7 +662,9 @@
   }
   generate_service_client(tservice);
   generate_service_helpers(tservice);
-  generate_service_processor(tservice);
+  if (phps_) {
+    generate_service_processor(tservice);
+  }
 
   // Close service file
   f_service_ << "?>" << endl;
@@ -914,8 +992,13 @@
       t_type* atype = get_true_type((*a_iter)->get_type());
       string cast = type_to_cast(atype);
       string req = "$request['" + (*a_iter)->get_name() + "']";
-      f_service_ <<
-        indent() << "$" << (*a_iter)->get_name() << " = isset(" << req << ") ? " << cast << req << " : null;" << endl;
+      if (atype->is_bool()) {
+        f_service_ <<
+          indent() << "$" << (*a_iter)->get_name() << " = " << cast << "(!empty(" << req << ") && (" << req << " !== 'false'));" << endl;
+      } else {
+        f_service_ <<
+          indent() << "$" << (*a_iter)->get_name() << " = isset(" << req << ") ? " << cast << req << " : null;" << endl;
+      }
       if (atype->is_string() &&
           ((t_base_type*)atype)->is_string_list()) {
         f_service_ <<
diff --git a/compiler/cpp/src/generate/t_php_generator.h b/compiler/cpp/src/generate/t_php_generator.h
index 1f2e258..5742747 100644
--- a/compiler/cpp/src/generate/t_php_generator.h
+++ b/compiler/cpp/src/generate/t_php_generator.h
@@ -21,13 +21,14 @@
  */
 class t_php_generator : public t_oop_generator {
  public:
-  t_php_generator(t_program* program, bool binary_inline=false, bool rest=false) :
+  t_php_generator(t_program* program, bool binary_inline=false, bool rest=false, bool phps=false) :
     t_oop_generator(program),
     binary_inline_(binary_inline),
-    rest_(rest) {
+    rest_(rest),
+    phps_(phps) {
     out_dir_base_ = (binary_inline_ ? "gen-phpi" : "gen-php");
   }
-  
+
   /**
    * Init and close methods
    */
@@ -58,6 +59,9 @@
   void generate_php_struct_writer(std::ofstream& out, t_struct* tstruct);
   void generate_php_function_helpers(t_function* tfunction);
 
+  void generate_php_type_spec(std::ofstream &out, t_type* t);
+  void generate_php_struct_spec(std::ofstream &out, t_struct* tstruct);
+
   /**
    * Service-level generation functions
    */
@@ -74,18 +78,18 @@
    */
 
   void generate_deserialize_field        (std::ofstream &out,
-                                          t_field*    tfield, 
+                                          t_field*    tfield,
                                           std::string prefix="",
                                           bool inclass=false);
-  
+
   void generate_deserialize_struct       (std::ofstream &out,
                                           t_struct*   tstruct,
                                           std::string prefix="");
-  
+
   void generate_deserialize_container    (std::ofstream &out,
                                           t_type*     ttype,
                                           std::string prefix="");
-  
+
   void generate_deserialize_set_element  (std::ofstream &out,
                                           t_set*      tset,
                                           std::string prefix="");
@@ -159,6 +163,11 @@
    */
   bool rest_;
 
+  /**
+   * Generate stubs for a PHP server
+   */
+  bool phps_;
+
 };
 
 #endif
diff --git a/compiler/cpp/src/main.cc b/compiler/cpp/src/main.cc
index 468a7b0..c633fac 100644
--- a/compiler/cpp/src/main.cc
+++ b/compiler/cpp/src/main.cc
@@ -139,6 +139,7 @@
 bool gen_xsd = false;
 bool gen_php = false;
 bool gen_phpi = false;
+bool gen_phps = true;
 bool gen_rest = false;
 bool gen_perl = false;
 bool gen_erl = false;
@@ -559,6 +560,8 @@
   fprintf(stderr, "  -javabean   Generate Java bean-style output files\n");
   fprintf(stderr, "  -php        Generate PHP output files\n");
   fprintf(stderr, "  -phpi       Generate PHP inlined files\n");
+  fprintf(stderr, "  -phps       Generate PHP server stubs (with -php)\n");
+  fprintf(stderr, "  -phpl       Generate PHP-lite (with -php)\n");
   fprintf(stderr, "  -py         Generate Python output files\n");
   fprintf(stderr, "  -rb         Generate Ruby output files\n");
   fprintf(stderr, "  -xsd        Generate XSD output files\n");
@@ -775,7 +778,7 @@
     for (size_t i = 0; i < includes.size(); ++i) {
       // Propogate output path from parent to child programs
       includes[i]->set_out_path(program->get_out_path());
-    
+
       generate(includes[i]);
     }
   }
@@ -810,7 +813,7 @@
 
     if (gen_php) {
       pverbose("Generating PHP\n");
-      t_php_generator* php = new t_php_generator(program, false, gen_rest);
+      t_php_generator* php = new t_php_generator(program, false, gen_rest, gen_phps);
       php->generate_program();
       delete php;
     }
@@ -940,6 +943,16 @@
         gen_php = true;
       } else if (strcmp(arg, "-phpi") == 0) {
         gen_phpi = true;
+      } else if (strcmp(arg, "-phps") == 0) {
+        if (!gen_php) {
+          gen_php = true;
+        }
+        gen_phps = true;
+      } else if (strcmp(arg, "-phpl") == 0) {
+        if (!gen_php) {
+          gen_php = true;
+        }
+        gen_phps = false;
       } else if (strcmp(arg, "-rest") == 0) {
         gen_rest = true;
       } else if (strcmp(arg, "-py") == 0) {
@@ -972,7 +985,7 @@
         if (arg == NULL) {
           fprintf(stderr, "-o: missing output directory");
           usage();
-        } 
+        }
         out_path = arg;
         struct stat sb;
         if (stat(out_path.c_str(), &sb) < 0) {
diff --git a/compiler/cpp/src/parse/t_base_type.h b/compiler/cpp/src/parse/t_base_type.h
index 929bf74..a440380 100644
--- a/compiler/cpp/src/parse/t_base_type.h
+++ b/compiler/cpp/src/parse/t_base_type.h
@@ -37,7 +37,7 @@
     base_(base),
     string_list_(false),
     string_enum_(false) {}
-    
+
   t_base get_base() const {
     return base_;
   }
@@ -49,7 +49,11 @@
   bool is_string() const {
     return base_ == TYPE_STRING;
   }
-  
+
+  bool is_bool() const {
+    return base_ == TYPE_BOOL;
+  }
+
   void set_string_list(bool val) {
     string_list_ = val;
   }
@@ -61,7 +65,7 @@
   void set_binary(bool val) {
     binary_ = val;
   }
-  
+
   bool is_binary() const {
     return (base_ == TYPE_STRING) && binary_;
   }
diff --git a/compiler/cpp/src/parse/t_type.h b/compiler/cpp/src/parse/t_type.h
index 3144356..b9da4fd 100644
--- a/compiler/cpp/src/parse/t_type.h
+++ b/compiler/cpp/src/parse/t_type.h
@@ -40,6 +40,7 @@
   virtual bool is_void()      const { return false; }
   virtual bool is_base_type() const { return false; }
   virtual bool is_string()    const { return false; }
+  virtual bool is_bool()      const { return false; }
   virtual bool is_typedef()   const { return false; }
   virtual bool is_enum()      const { return false; }
   virtual bool is_struct()    const { return false; }