Revert #4529 (#5008)

* Revert " Optimize ‘hist’ for multi-core CPU (#4529)"

This reverts commit 4d6590be3c.

* Fix build

src/tree/param.h

@@ -281,7 +281,7 @@ XGBOOST_DEVICE inline T CalcGain(const TrainingParams &p, T sum_grad, T sum_hess
     }
   } else {
     T w = CalcWeight(p, sum_grad, sum_hess);
-    T ret = CalcGainGivenWeight<TrainingParams, T>(p, sum_grad, sum_hess, w);
+    T ret = CalcGainGivenWeight(p, sum_grad, sum_hess, w);
     if (p.reg_alpha == 0.0f) {
       return ret;
     } else {
@@ -301,7 +301,7 @@ template <typename TrainingParams, typename T>
 XGBOOST_DEVICE inline T CalcGain(const TrainingParams &p, T sum_grad, T sum_hess,
                                  T test_grad, T test_hess) {
   T w = CalcWeight(sum_grad, sum_hess);
-  T ret = CalcGainGivenWeight<TrainingParams, T>(p, test_grad, test_hess);
+  T ret = CalcGainGivenWeight(p, test_grad, test_hess);
   if (p.reg_alpha == 0.0f) {
     return ret;
   } else {
@@ -340,16 +340,15 @@ XGBOOST_DEVICE inline float CalcWeight(const TrainingParams &p, GpairT sum_grad)
 }
 
 /*! \brief core statistics used for tree construction */
-struct  GradStats {
-  typedef double GradType;
+struct XGBOOST_ALIGNAS(16) GradStats {
   /*! \brief sum gradient statistics */
-  GradType sum_grad;
+  double sum_grad;
   /*! \brief sum hessian statistics */
-  GradType sum_hess;
+  double sum_hess;
 
  public:
-  XGBOOST_DEVICE GradType GetGrad() const { return sum_grad; }
-  XGBOOST_DEVICE GradType GetHess() const { return sum_hess; }
+  XGBOOST_DEVICE double GetGrad() const { return sum_grad; }
+  XGBOOST_DEVICE double GetHess() const { return sum_hess; }
 
   XGBOOST_DEVICE GradStats() : sum_grad{0}, sum_hess{0} {
     static_assert(sizeof(GradStats) == 16,
@@ -359,7 +358,7 @@ struct  GradStats {
   template <typename GpairT>
   XGBOOST_DEVICE explicit GradStats(const GpairT &sum)
       : sum_grad(sum.GetGrad()), sum_hess(sum.GetHess()) {}
-  explicit GradStats(const GradType grad, const GradType hess)
+  explicit GradStats(const double grad, const double hess)
       : sum_grad(grad), sum_hess(hess) {}
   /*!
    * \brief accumulate statistics
@@ -384,7 +383,7 @@ struct  GradStats {
   /*! \return whether the statistics is not used yet */
   inline bool Empty() const { return sum_hess == 0.0; }
   /*! \brief add statistics to the data */
-  inline void Add(GradType grad, GradType hess) {
+  inline void Add(double grad, double hess) {
     sum_grad += grad;
     sum_hess += hess;
   }
@@ -402,7 +401,6 @@ struct SplitEntry {
   bst_float split_value{0.0f};
   GradStats left_sum;
   GradStats right_sum;
-  bool default_left{true};
 
   /*! \brief constructor */
   SplitEntry()  = default;
@@ -417,11 +415,7 @@ struct SplitEntry {
    * \param split_index the feature index where the split is on
    */
   inline bool NeedReplace(bst_float new_loss_chg, unsigned split_index) const {
-    if (!std::isfinite(new_loss_chg)) {  // in some cases new_loss_chg can be NaN or Inf,
-                                         // for example when lambda = 0 & min_child_weight = 0
-                                         // skip value in this case
-      return false;
-    } else if (this->SplitIndex() <= split_index) {
+    if (this->SplitIndex() <= split_index) {
       return new_loss_chg > this->loss_chg;
     } else {
       return !(this->loss_chg > new_loss_chg);
@@ -439,7 +433,6 @@ struct SplitEntry {
       this->split_value = e.split_value;
       this->left_sum = e.left_sum;
       this->right_sum = e.right_sum;
-      this->default_left = e.default_left;
       return true;
     } else {
       return false;
@@ -454,11 +447,13 @@ struct SplitEntry {
    * \return whether the proposed split is better and can replace current split
    */
   inline bool Update(bst_float new_loss_chg, unsigned split_index,
-                     bst_float new_split_value, bool new_default_left,
+                     bst_float new_split_value, bool default_left,
                      const GradStats &left_sum, const GradStats &right_sum) {
     if (this->NeedReplace(new_loss_chg, split_index)) {
       this->loss_chg = new_loss_chg;
-      this->default_left = new_default_left;
+      if (default_left) {
+        split_index |= (1U << 31);
+      }
       this->sindex = split_index;
       this->split_value = new_split_value;
       this->left_sum = left_sum;
@@ -474,9 +469,9 @@ struct SplitEntry {
     dst.Update(src);
   }
   /*!\return feature index to split on */
-  inline unsigned SplitIndex() const { return sindex; }
+  inline unsigned SplitIndex() const { return sindex & ((1U << 31) - 1U); }
   /*!\return whether missing value goes to left branch */
-  inline bool DefaultLeft() const { return default_left; }
+  inline bool DefaultLeft() const { return (sindex >> 31) != 0; }
 };
 
 }  // namespace tree

src/tree/split_evaluator.cc

@@ -284,9 +284,7 @@ class MonotonicConstraint final : public SplitEvaluator {
                 bst_float leftweight,
                 bst_float rightweight) override {
     inner_->AddSplit(nodeid, leftid, rightid, featureid, leftweight, rightweight);
-
-    bst_uint newsize = std::max(bst_uint(lower_.size()), bst_uint(std::max(leftid, rightid) + 1u));
-
+    bst_uint newsize = std::max(leftid, rightid) + 1;
     lower_.resize(newsize);
     upper_.resize(newsize);
     bst_int constraint = GetConstraint(featureid);

src/tree/updater_quantile_hist.h

@@ -1,8 +1,8 @@
 /*!
- * Copyright 2017-2019 by Contributors
+ * Copyright 2017-2018 by Contributors
  * \file updater_quantile_hist.h
  * \brief use quantized feature values to construct a tree
- * \author Philip Cho, Tianqi Chen, Egor Smirnov
+ * \author Philip Cho, Tianqi Chen
  */
 #ifndef XGBOOST_TREE_UPDATER_QUANTILE_HIST_H_
 #define XGBOOST_TREE_UPDATER_QUANTILE_HIST_H_
@@ -18,19 +18,51 @@
 #include <iomanip>
 #include <unordered_map>
 #include <utility>
-#include <tuple>
 
 #include "./param.h"
 #include "./split_evaluator.h"
 #include "../common/random.h"
+#include "../common/timer.h"
 #include "../common/hist_util.h"
 #include "../common/row_set.h"
 #include "../common/column_matrix.h"
 
 namespace xgboost {
-namespace common {
-  struct GradStatHist;
-}
+
+/*!
+ * \brief A C-style array with in-stack allocation. As long as the array is smaller than MaxStackSize, it will be allocated inside the stack. Otherwise, it will be heap-allocated.
+ */
+template<typename T, size_t MaxStackSize>
+class MemStackAllocator {
+ public:
+  explicit MemStackAllocator(size_t required_size): required_size_(required_size) {
+  }
+
+  T* Get() {
+    if (!ptr_) {
+      if (MaxStackSize >= required_size_) {
+        ptr_ = stack_mem_;
+      } else {
+        ptr_ =  reinterpret_cast<T*>(malloc(required_size_ * sizeof(T)));
+        do_free_ = true;
+      }
+    }
+
+    return ptr_;
+  }
+
+  ~MemStackAllocator() {
+    if (do_free_) free(ptr_);
+  }
+
+
+ private:
+  T* ptr_ = nullptr;
+  bool do_free_ = false;
+  size_t required_size_;
+  T stack_mem_[MaxStackSize];
+};
+
 namespace tree {
 
 using xgboost::common::GHistIndexMatrix;
@@ -71,7 +103,6 @@ class QuantileHistMaker: public TreeUpdater {
   bool is_gmat_initialized_;
 
   // data structure
- public:
   struct NodeEntry {
     /*! \brief statics for node entry */
     GradStats stats;
@@ -83,8 +114,7 @@ class QuantileHistMaker: public TreeUpdater {
     SplitEntry best;
     // constructor
     explicit NodeEntry(const TrainParam& param)
-        : root_gain(0.0f), weight(0.0f) {
-    }
+        : root_gain(0.0f), weight(0.0f) {}
   };
   // actual builder that runs the algorithm
 
@@ -94,8 +124,11 @@ class QuantileHistMaker: public TreeUpdater {
     explicit Builder(const TrainParam& param,
                      std::unique_ptr<TreeUpdater> pruner,
                      std::unique_ptr<SplitEvaluator> spliteval)
-      : param_(param), pruner_(std::move(pruner)), spliteval_(std::move(spliteval)),
-      p_last_tree_(nullptr), p_last_fmat_(nullptr) {  }
+      : param_(param), pruner_(std::move(pruner)),
+        spliteval_(std::move(spliteval)), p_last_tree_(nullptr),
+        p_last_fmat_(nullptr) {
+      builder_monitor_.Init("Quantile::Builder");
+    }
     // update one tree, growing
     virtual void Update(const GHistIndexMatrix& gmat,
                         const GHistIndexBlockMatrix& gmatb,
@@ -104,104 +137,42 @@ class QuantileHistMaker: public TreeUpdater {
                         DMatrix* p_fmat,
                         RegTree* p_tree);
 
+    inline void BuildHist(const std::vector<GradientPair>& gpair,
+                          const RowSetCollection::Elem row_indices,
+                          const GHistIndexMatrix& gmat,
+                          const GHistIndexBlockMatrix& gmatb,
+                          GHistRow hist,
+                          bool sync_hist) {
+      builder_monitor_.Start("BuildHist");
+      if (param_.enable_feature_grouping > 0) {
+        hist_builder_.BuildBlockHist(gpair, row_indices, gmatb, hist);
+      } else {
+        hist_builder_.BuildHist(gpair, row_indices, gmat, hist);
+      }
+      if (sync_hist) {
+        this->histred_.Allreduce(hist.data(), hist_builder_.GetNumBins());
+      }
+      builder_monitor_.Stop("BuildHist");
+    }
+
+    inline void SubtractionTrick(GHistRow self, GHistRow sibling, GHistRow parent) {
+      builder_monitor_.Start("SubtractionTrick");
+      hist_builder_.SubtractionTrick(self, sibling, parent);
+      builder_monitor_.Stop("SubtractionTrick");
+    }
+
     bool UpdatePredictionCache(const DMatrix* data,
                                HostDeviceVector<bst_float>* p_out_preds);
 
-    std::tuple<common::GradStatHist::GradType*, common::GradStatHist*>
-    GetHistBuffer(std::vector<uint8_t>* hist_is_init,
-      std::vector<common::GradStatHist>* grad_stats, size_t block_id, size_t nthread,
-      size_t tid, std::vector<common::GradStatHist::GradType*>* data_hist, size_t hist_size);
-
    protected:
     /* tree growing policies */
     struct ExpandEntry {
       int nid;
-      int sibling_nid;
-      int parent_nid;
       int depth;
       bst_float loss_chg;
       unsigned timestamp;
-      ExpandEntry(int nid, int sibling_nid, int parent_nid, int depth, bst_float loss_chg,
-        unsigned tstmp) : nid(nid), sibling_nid(sibling_nid), parent_nid(parent_nid),
-        depth(depth), loss_chg(loss_chg), timestamp(tstmp) {}
-    };
-
-    struct TreeGrowingPerfMonitor {
-      enum timer_name {INIT_DATA, INIT_NEW_NODE, BUILD_HIST, EVALUATE_SPLIT, APPLY_SPLIT};
-
-      double global_start;
-
-      // performance counters
-      double tstart;
-      double time_init_data = 0;
-      double time_init_new_node = 0;
-      double time_build_hist = 0;
-      double time_evaluate_split = 0;
-      double time_apply_split = 0;
-
-      inline void StartPerfMonitor() {
-        global_start = dmlc::GetTime();
-      }
-
-      inline void EndPerfMonitor() {
-        CHECK_GT(global_start, 0);
-        double total_time = dmlc::GetTime() - global_start;
-        LOG(INFO) << "\nInitData:          "
-                  << std::fixed << std::setw(6) << std::setprecision(4) << time_init_data
-                  << " (" << std::fixed << std::setw(5) << std::setprecision(2)
-                  << time_init_data / total_time * 100 << "%)\n"
-                  << "InitNewNode:       "
-                  << std::fixed << std::setw(6) << std::setprecision(4) << time_init_new_node
-                  << " (" << std::fixed << std::setw(5) << std::setprecision(2)
-                  << time_init_new_node / total_time * 100 << "%)\n"
-                  << "BuildHist:         "
-                  << std::fixed << std::setw(6) << std::setprecision(4) << time_build_hist
-                  << " (" << std::fixed << std::setw(5) << std::setprecision(2)
-                  << time_build_hist / total_time * 100 << "%)\n"
-                  << "EvaluateSplit:     "
-                  << std::fixed << std::setw(6) << std::setprecision(4) << time_evaluate_split
-                  << " (" << std::fixed << std::setw(5) << std::setprecision(2)
-                  << time_evaluate_split / total_time * 100 << "%)\n"
-                  << "ApplySplit:        "
-                  << std::fixed << std::setw(6) << std::setprecision(4) << time_apply_split
-                  << " (" << std::fixed << std::setw(5) << std::setprecision(2)
-                  << time_apply_split / total_time * 100 << "%)\n"
-                  << "========================================\n"
-                  << "Total:             "
-                  << std::fixed << std::setw(6) << std::setprecision(4) << total_time << std::endl;
-        // clear performance counters
-        time_init_data = 0;
-        time_init_new_node = 0;
-        time_build_hist = 0;
-        time_evaluate_split = 0;
-        time_apply_split = 0;
-      }
-
-      inline void TickStart() {
-        tstart = dmlc::GetTime();
-      }
-
-      inline void UpdatePerfTimer(const timer_name &timer_name) {
-        // CHECK_GT(tstart, 0); // TODO Fix
-        switch (timer_name) {
-          case INIT_DATA:
-            time_init_data += dmlc::GetTime() - tstart;
-            break;
-          case INIT_NEW_NODE:
-            time_init_new_node += dmlc::GetTime() - tstart;
-            break;
-          case BUILD_HIST:
-            time_build_hist += dmlc::GetTime() - tstart;
-            break;
-          case EVALUATE_SPLIT:
-            time_evaluate_split += dmlc::GetTime() - tstart;
-            break;
-          case APPLY_SPLIT:
-            time_apply_split += dmlc::GetTime() - tstart;
-            break;
-        }
-        tstart = -1;
-      }
+      ExpandEntry(int nid, int depth, bst_float loss_chg, unsigned tstmp)
+              : nid(nid), depth(depth), loss_chg(loss_chg), timestamp(tstmp) {}
     };
 
     // initialize temp data structure
@@ -210,16 +181,43 @@ class QuantileHistMaker: public TreeUpdater {
                   const DMatrix& fmat,
                   const RegTree& tree);
 
+    void EvaluateSplit(const int nid,
+                       const GHistIndexMatrix& gmat,
+                       const HistCollection& hist,
+                       const DMatrix& fmat,
+                       const RegTree& tree);
+
+    void ApplySplit(int nid,
+                    const GHistIndexMatrix& gmat,
+                    const ColumnMatrix& column_matrix,
+                    const HistCollection& hist,
+                    const DMatrix& fmat,
+                    RegTree* p_tree);
+
+    void ApplySplitDenseData(const RowSetCollection::Elem rowset,
+                             const GHistIndexMatrix& gmat,
+                             std::vector<RowSetCollection::Split>* p_row_split_tloc,
+                             const Column& column,
+                             bst_int split_cond,
+                             bool default_left);
+
+    void ApplySplitSparseData(const RowSetCollection::Elem rowset,
+                              const GHistIndexMatrix& gmat,
+                              std::vector<RowSetCollection::Split>* p_row_split_tloc,
+                              const Column& column,
+                              bst_uint lower_bound,
+                              bst_uint upper_bound,
+                              bst_int split_cond,
+                              bool default_left);
+
     void InitNewNode(int nid,
                      const GHistIndexMatrix& gmat,
                      const std::vector<GradientPair>& gpair,
                      const DMatrix& fmat,
-                     RegTree* tree,
-                     QuantileHistMaker::NodeEntry* snode,
-                     int32_t parentid);
+                     const RegTree& tree);
 
     // enumerate the split values of specific feature
-    bool EnumerateSplit(int d_step,
+    void EnumerateSplit(int d_step,
                         const GHistIndexMatrix& gmat,
                         const GHistRow& hist,
                         const NodeEntry& snode,
@@ -228,36 +226,37 @@ class QuantileHistMaker: public TreeUpdater {
                         bst_uint fid,
                         bst_uint nodeID);
 
-    void EvaluateSplitsBatch(const std::vector<ExpandEntry>& nodes,
-          const GHistIndexMatrix& gmat,
-          const DMatrix& fmat,
-          const std::vector<std::vector<uint8_t>>& hist_is_init,
-          const std::vector<std::vector<common::GradStatHist::GradType*>>& hist_buffers);
+    void ExpandWithDepthWise(const GHistIndexMatrix &gmat,
+                             const GHistIndexBlockMatrix &gmatb,
+                             const ColumnMatrix &column_matrix,
+                             DMatrix *p_fmat,
+                             RegTree *p_tree,
+                             const std::vector<GradientPair> &gpair_h);
 
-    void ReduceHistograms(
-        common::GradStatHist::GradType* hist_data,
-        common::GradStatHist::GradType* sibling_hist_data,
-        common::GradStatHist::GradType* parent_hist_data,
-        const size_t ibegin,
-        const size_t iend,
-        const size_t inode,
-        const std::vector<std::vector<uint8_t>>& hist_is_init,
-        const std::vector<std::vector<common::GradStatHist::GradType*>>& hist_buffers);
-
-    void SyncHistograms(
-        RegTree* p_tree,
-        const std::vector<ExpandEntry>& nodes,
-        std::vector<std::vector<common::GradStatHist::GradType*>>* hist_buffers,
-        std::vector<std::vector<uint8_t>>* hist_is_init,
-        const std::vector<std::vector<common::GradStatHist>>& grad_stats);
-
-     void ExpandWithDepthWise(const GHistIndexMatrix &gmat,
+    void BuildLocalHistograms(int *starting_index,
+                              int *sync_count,
+                              const GHistIndexMatrix &gmat,
                               const GHistIndexBlockMatrix &gmatb,
-                              const ColumnMatrix &column_matrix,
-                              DMatrix *p_fmat,
                               RegTree *p_tree,
                               const std::vector<GradientPair> &gpair_h);
 
+    void SyncHistograms(int starting_index,
+                        int sync_count,
+                        RegTree *p_tree);
+
+    void BuildNodeStats(const GHistIndexMatrix &gmat,
+                        DMatrix *p_fmat,
+                        RegTree *p_tree,
+                        const std::vector<GradientPair> &gpair_h);
+
+    void EvaluateSplits(const GHistIndexMatrix &gmat,
+                        const ColumnMatrix &column_matrix,
+                        DMatrix *p_fmat,
+                        RegTree *p_tree,
+                        int *num_leaves,
+                        int depth,
+                        unsigned *timestamp,
+                        std::vector<ExpandEntry> *temp_qexpand_depth);
 
     void ExpandWithLossGuide(const GHistIndexMatrix& gmat,
                              const GHistIndexBlockMatrix& gmatb,
@@ -266,62 +265,6 @@ class QuantileHistMaker: public TreeUpdater {
                              RegTree* p_tree,
                              const std::vector<GradientPair>& gpair_h);
 
-
-    void BuildHistsBatch(const std::vector<ExpandEntry>& nodes, RegTree* tree,
-      const GHistIndexMatrix &gmat, const std::vector<GradientPair>& gpair,
-      std::vector<std::vector<common::GradStatHist::GradType*>>* hist_buffers,
-      std::vector<std::vector<uint8_t>>* hist_is_init);
-
-    void BuildNodeStat(const GHistIndexMatrix &gmat,
-                        DMatrix *p_fmat,
-                        RegTree *p_tree,
-                        const std::vector<GradientPair> &gpair_h,
-                        int32_t nid);
-
-    void BuildNodeStatBatch(
-        const GHistIndexMatrix &gmat,
-        DMatrix *p_fmat,
-        RegTree *p_tree,
-        const std::vector<GradientPair> &gpair_h,
-        const std::vector<ExpandEntry>& nodes);
-
-    int32_t FindSplitCond(int32_t nid,
-                          RegTree *p_tree,
-                          const GHistIndexMatrix &gmat);
-
-    void CreateNewNodesBatch(
-        const std::vector<ExpandEntry>& nodes,
-        const GHistIndexMatrix &gmat,
-        const ColumnMatrix &column_matrix,
-        DMatrix *p_fmat,
-        RegTree *p_tree,
-        int *num_leaves,
-        int depth,
-        unsigned *timestamp,
-        std::vector<ExpandEntry> *temp_qexpand_depth);
-
-    template<typename TaskType, typename NodeType>
-    void CreateTasksForApplySplit(
-          const std::vector<ExpandEntry>& nodes,
-          const GHistIndexMatrix &gmat,
-          RegTree *p_tree,
-          int *num_leaves,
-          const int depth,
-          const size_t block_size,
-          std::vector<TaskType>* tasks,
-          std::vector<NodeType>* nodes_bounds);
-
-    void CreateTasksForBuildHist(
-        size_t block_size_rows,
-        size_t nthread,
-        const std::vector<ExpandEntry>& nodes,
-        std::vector<std::vector<common::GradStatHist::GradType*>>* hist_buffers,
-        std::vector<std::vector<uint8_t>>* hist_is_init,
-        std::vector<std::vector<common::GradStatHist>>* grad_stats,
-        std::vector<int32_t>* task_nid,
-        std::vector<int32_t>* task_node_idx,
-        std::vector<int32_t>* task_block_idx);
-
     inline static bool LossGuide(ExpandEntry lhs, ExpandEntry rhs) {
       if (lhs.loss_chg == rhs.loss_chg) {
         return lhs.timestamp > rhs.timestamp;  // favor small timestamp
@@ -330,8 +273,6 @@ class QuantileHistMaker: public TreeUpdater {
       }
     }
 
-    HistCollection hist_buff_;
-
     //  --data fields--
     const TrainParam& param_;
     // number of omp thread used during training
@@ -342,7 +283,6 @@ class QuantileHistMaker: public TreeUpdater {
     // the temp space for split
     std::vector<RowSetCollection::Split> row_split_tloc_;
     std::vector<SplitEntry> best_split_tloc_;
-    std::vector<size_t> buffer_for_partition_;
     /*! \brief TreeNode Data: statistics for each constructed node */
     std::vector<NodeEntry> snode_;
     /*! \brief culmulative histogram of gradients. */
@@ -374,8 +314,8 @@ class QuantileHistMaker: public TreeUpdater {
     enum DataLayout { kDenseDataZeroBased, kDenseDataOneBased, kSparseData };
     DataLayout data_layout_;
 
-    TreeGrowingPerfMonitor perf_monitor;
-    rabit::Reducer<common::GradStatHist, common::GradStatHist::Reduce> histred_;
+    common::Monitor builder_monitor_;
+    rabit::Reducer<GradStats, GradStats::Reduce> histred_;
   };
 
   std::unique_ptr<Builder> builder_;