Potential slowness in operands_and_partials

Developers
20

OMG I found it. So embarrassing. This must be leftover from a previous design that never made it in:

--- a/stan/math/prim/mat/meta/operands_and_partials.hpp
+++ b/stan/math/prim/mat/meta/operands_and_partials.hpp
@@ -22,7 +22,7 @@ class ops_partials_edge<ViewElt, Eigen::Matrix<Op, R, C>> {
   partials_t partials_;
   empty_broadcast_array<partials_t, Eigen::Matrix<Op, R, C>> partials_vec_;
   ops_partials_edge() {}
-  explicit ops_partials_edge(const Eigen::Matrix<Op, R, C> ops) {}
+  explicit ops_partials_edge(const Eigen::Matrix<Op, R, C>& ops) {}
 
  private:
   template <typename, typename, typename, typename, typename, typename>
@@ -40,7 +40,7 @@ class ops_partials_edge<ViewElt, std::vector<Eigen::Matrix<Op, R, C>>> {
   typedef empty_broadcast_array<ViewElt, Eigen::Matrix<Op, R, C>> partials_t;
   empty_broadcast_array<partials_t, Eigen::Matrix<Op, R, C>> partials_vec_;
   ops_partials_edge() {}
-  explicit ops_partials_edge(const std::vector<Eigen::Matrix<Op, R, C>> ops) {}
+  explicit ops_partials_edge(const std::vector<Eigen::Matrix<Op, R, C>>& ops) {}
 
  private:
   template <typename, typename, typename, typename, typename, typename>
@@ -60,7 +60,7 @@ class ops_partials_edge<ViewElt, std::vector<std::vector<Op>>> {
   partials_t partials_;
   empty_broadcast_array<partials_t, std::vector<std::vector<Op>>> partials_vec_;
   ops_partials_edge() {}
-  explicit ops_partials_edge(const std::vector<std::vector<Op>> ops) {}
+  explicit ops_partials_edge(const std::vector<std::vector<Op>>& ops) {}

This change removes operands_and_partials constructor from my list, at least. I’m also using -O3, though I was also experimenting with -g -fomit-frame-pointer at one point.

@tadej It does create a new matrix, but the compiler essentially creates that in partials_, see https://isocpp.org/wiki/faq/ctors#init-lists

@bbbales2 Which checks can we remove from the GLMs? I wasn’t totally following.

[edit - with this change I now see @tadej’s normal GLM benchmark running in about 56% of the time it takes on `develop - close to recovering that 2x, though it seems like maybe there is another 2x in there?]

4 Likes