Speed-up "Split" and some code refactorings (#2883)

* commit

* fix msvc

* fix format

include/LightGBM/bin.h

@@ -218,61 +218,6 @@ class BinMapper {
   uint32_t most_freq_bin_;
 };
 
-/*!
-* \brief Interface for ordered bin data. efficient for construct histogram, especially for sparse bin
-*        There are 2 advantages by using ordered bin.
-*        1. group the data by leafs to improve the cache hit.
-*        2. only store the non-zero bin, which can speed up the histogram construction for sparse features.
-*        However it brings additional cost: it need re-order the bins after every split, which will cost much for dense feature.
-*        So we only using ordered bin for sparse situations.
-*/
-class OrderedBin {
- public:
-  /*! \brief virtual destructor */
-  virtual ~OrderedBin() {}
-
-  /*!
-  * \brief Initialization logic.
-  * \param used_indices If used_indices.size() == 0 means using all data, otherwise, used_indices[i] == true means i-th data is used
-           (this logic was build for bagging logic)
-  * \param num_leaves Number of leaves on this iteration
-  */
-  virtual void Init(const char* used_indices, data_size_t num_leaves) = 0;
-
-  /*!
-  * \brief Construct histogram by using this bin
-  *        Note: Unlike Bin, OrderedBin doesn't use ordered gradients and ordered hessians.
-  *        Because it is hard to know the relative index in one leaf for sparse bin, since we skipped zero bins.
-  * \param leaf Using which leaf's data to construct
-  * \param gradients Gradients, Note:non-ordered by leaf
-  * \param hessians Hessians, Note:non-ordered by leaf
-  * \param out Output Result
-  */
-  virtual void ConstructHistogram(int leaf, const score_t* gradients,
-    const score_t* hessians, hist_t* out) const = 0;
-
-  /*!
-  * \brief Construct histogram by using this bin
-  *        Note: Unlike Bin, OrderedBin doesn't use ordered gradients and ordered hessians.
-  *        Because it is hard to know the relative index in one leaf for sparse bin, since we skipped zero bins.
-  * \param leaf Using which leaf's data to construct
-  * \param gradients Gradients, Note:non-ordered by leaf
-  * \param out Output Result
-  */
-  virtual void ConstructHistogram(int leaf, const score_t* gradients, hist_t* out) const = 0;
-
-  /*!
-  * \brief Split current bin, and perform re-order by leaf
-  * \param leaf Using which leaf's to split
-  * \param right_leaf The new leaf index after perform this split
-  * \param is_in_leaf is_in_leaf[i] == mark means the i-th data will be on left leaf after split
-  * \param mark is_in_leaf[i] == mark means the i-th data will be on left leaf after split
-  */
-  virtual void Split(int leaf, int right_leaf, const char* is_in_leaf, char mark) = 0;
-
-  virtual data_size_t NonZeroCount(int leaf) const = 0;
-};
-
 /*! \brief Iterator for one bin column */
 class BinIterator {
  public:
@@ -382,43 +327,33 @@ class Bin {
   virtual void ConstructHistogram(data_size_t start, data_size_t end,
                                   const score_t* ordered_gradients, hist_t* out) const = 0;
 
-  /*!
-  * \brief Split data according to threshold, if bin <= threshold, will put into left(lte_indices), else put into right(gt_indices)
-  * \param min_bin min_bin of current used feature
-  * \param max_bin max_bin of current used feature
-  * \param default_bin default bin for feature value 0
-  * \param most_freq_bin
-  * \param missing_type missing type
-  * \param default_left missing bin will go to left child
-  * \param threshold The split threshold.
-  * \param data_indices Used data indices. After called this function. The less than or equal data indices will store on this object.
-  * \param num_data Number of used data
-  * \param lte_indices After called this function. The less or equal data indices will store on this object.
-  * \param gt_indices After called this function. The greater data indices will store on this object.
-  * \return The number of less than or equal data.
-  */
   virtual data_size_t Split(uint32_t min_bin, uint32_t max_bin,
-    uint32_t default_bin, uint32_t most_freq_bin, MissingType missing_type, bool default_left, uint32_t threshold,
-    data_size_t* data_indices, data_size_t num_data,
-    data_size_t* lte_indices, data_size_t* gt_indices) const = 0;
+                            uint32_t default_bin, uint32_t most_freq_bin,
+                            MissingType missing_type, bool default_left,
+                            uint32_t threshold, const data_size_t* data_indices,
+                            data_size_t cnt,
+                            data_size_t* lte_indices,
+                            data_size_t* gt_indices) const = 0;
+
+  virtual data_size_t SplitCategorical(
+      uint32_t min_bin, uint32_t max_bin, uint32_t most_freq_bin,
+      const uint32_t* threshold, int num_threshold,
+      const data_size_t* data_indices, data_size_t cnt,
+      data_size_t* lte_indices, data_size_t* gt_indices) const = 0;
+
+  virtual data_size_t Split(uint32_t max_bin, uint32_t default_bin,
+                            uint32_t most_freq_bin, MissingType missing_type,
+                            bool default_left, uint32_t threshold,
+                            const data_size_t* data_indices, data_size_t cnt,
+                            data_size_t* lte_indices,
+                            data_size_t* gt_indices) const = 0;
+
+  virtual data_size_t SplitCategorical(
+      uint32_t max_bin, uint32_t most_freq_bin, const uint32_t* threshold,
+      int num_threshold, const data_size_t* data_indices, data_size_t cnt,
+      data_size_t* lte_indices, data_size_t* gt_indices) const = 0;
+
 
-  /*!
-  * \brief Split data according to threshold, if bin <= threshold, will put into left(lte_indices), else put into right(gt_indices)
-  * \param min_bin min_bin of current used feature
-  * \param max_bin max_bin of current used feature
-  * \param most_freq_bin
-  * \param threshold The split threshold.
-  * \param num_threshold Number of threshold
-  * \param data_indices Used data indices. After called this function. The less than or equal data indices will store on this object.
-  * \param num_data Number of used data
-  * \param lte_indices After called this function. The less or equal data indices will store on this object.
-  * \param gt_indices After called this function. The greater data indices will store on this object.
-  * \return The number of less than or equal data.
-  */
-  virtual data_size_t SplitCategorical(uint32_t min_bin, uint32_t max_bin,
-                            uint32_t most_freq_bin, const uint32_t* threshold, int num_threshold,
-                            data_size_t* data_indices, data_size_t num_data,
-                            data_size_t* lte_indices, data_size_t* gt_indices) const = 0;
 
   /*!
   * \brief After pushed all feature data, call this could have better refactor for bin data

include/LightGBM/dataset.h

@@ -535,13 +535,16 @@ class Dataset {
 
   void FixHistogram(int feature_idx, double sum_gradient, double sum_hessian, hist_t* data) const;
 
-  inline data_size_t Split(int feature,
-                           const uint32_t* threshold, int num_threshold,  bool default_left,
-                           data_size_t* data_indices, data_size_t num_data,
-                           data_size_t* lte_indices, data_size_t* gt_indices) const {
+  inline data_size_t Split(int feature, const uint32_t* threshold,
+                           int num_threshold, bool default_left,
+                           const data_size_t* data_indices, 
+                           data_size_t cnt, data_size_t* lte_indices,
+                           data_size_t* gt_indices) const {
     const int group = feature2group_[feature];
     const int sub_feature = feature2subfeature_[feature];
-    return feature_groups_[group]->Split(sub_feature, threshold, num_threshold, default_left, data_indices, num_data, lte_indices, gt_indices);
+    return feature_groups_[group]->Split(
+        sub_feature, threshold, num_threshold, default_left, data_indices,
+        cnt, lte_indices, gt_indices);
   }
 
   inline int SubFeatureBinOffset(int i) const {

include/LightGBM/feature_group.h

@@ -228,35 +228,50 @@ class FeatureGroup {
     return bin_data_->GetIterator(min_bin, max_bin, most_freq_bin);
   }
 
-  inline data_size_t Split(
-    int sub_feature,
-    const uint32_t* threshold,
-    int num_threshold,
-    bool default_left,
-    data_size_t* data_indices, data_size_t num_data,
-    data_size_t* lte_indices, data_size_t* gt_indices) const {
+  inline data_size_t Split(int sub_feature, const uint32_t* threshold,
+                           int num_threshold, bool default_left,
+                           const data_size_t* data_indices, data_size_t cnt,
+                           data_size_t* lte_indices,
+                           data_size_t* gt_indices) const {
     uint32_t default_bin = bin_mappers_[sub_feature]->GetDefaultBin();
     uint32_t most_freq_bin = bin_mappers_[sub_feature]->GetMostFreqBin();
     if (!is_multi_val_) {
       uint32_t min_bin = bin_offsets_[sub_feature];
       uint32_t max_bin = bin_offsets_[sub_feature + 1] - 1;
       if (bin_mappers_[sub_feature]->bin_type() == BinType::NumericalBin) {
         auto missing_type = bin_mappers_[sub_feature]->missing_type();
-        return bin_data_->Split(min_bin, max_bin, default_bin, most_freq_bin, missing_type, default_left,
-          *threshold, data_indices, num_data, lte_indices, gt_indices);
+        if (num_feature_ == 1) {
+          return bin_data_->Split(max_bin, default_bin, most_freq_bin,
+                                  missing_type, default_left, *threshold,
+                                  data_indices, cnt, lte_indices, gt_indices);
+        } else {
+          return bin_data_->Split(min_bin, max_bin, default_bin, most_freq_bin,
+                                  missing_type, default_left, *threshold,
+                                  data_indices, cnt, lte_indices, gt_indices);
+        }
       } else {
-        return bin_data_->SplitCategorical(min_bin, max_bin, most_freq_bin, threshold, num_threshold, data_indices, num_data, lte_indices, gt_indices);
+        if (num_feature_ == 1) {
+          return bin_data_->SplitCategorical(max_bin, most_freq_bin, threshold,
+                                             num_threshold, data_indices, cnt,
+                                             lte_indices, gt_indices);
+        } else {
+          return bin_data_->SplitCategorical(
+              min_bin, max_bin, most_freq_bin, threshold, num_threshold,
+              data_indices, cnt, lte_indices, gt_indices);
+        }
       }
     } else {
       int addi = bin_mappers_[sub_feature]->GetMostFreqBin() == 0 ? 0 : 1;
-      uint32_t min_bin = 1;
       uint32_t max_bin = bin_mappers_[sub_feature]->num_bin() - 1 + addi;
       if (bin_mappers_[sub_feature]->bin_type() == BinType::NumericalBin) {
         auto missing_type = bin_mappers_[sub_feature]->missing_type();
-        return multi_bin_data_[sub_feature]->Split(min_bin, max_bin, default_bin, most_freq_bin, missing_type, default_left,
-          *threshold, data_indices, num_data, lte_indices, gt_indices);
+        return multi_bin_data_[sub_feature]->Split(
+            max_bin, default_bin, most_freq_bin, missing_type, default_left,
+            *threshold, data_indices, cnt, lte_indices, gt_indices);
       } else {
-        return multi_bin_data_[sub_feature]->SplitCategorical(min_bin, max_bin, most_freq_bin, threshold, num_threshold, data_indices, num_data, lte_indices, gt_indices);
+        return multi_bin_data_[sub_feature]->SplitCategorical(
+            max_bin, most_freq_bin, threshold, num_threshold, data_indices, cnt,
+            lte_indices, gt_indices);
       }
     }
   }

include/LightGBM/meta.h

@@ -75,6 +75,9 @@ const int kAlignedSize = 32;
 
 #define SIZE_ALIGNED(t) ((t) + kAlignedSize - 1) / kAlignedSize * kAlignedSize
 
+// Refer to https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4127?view=vs-2019
+#pragma warning(disable : 4127)
+
 }  // namespace LightGBM
 
 #endif   // LightGBM_META_H_

src/io/bin.cpp

@@ -14,7 +14,6 @@
 #include <cstring>
 
 #include "dense_bin.hpp"
-#include "dense_nbits_bin.hpp"
 #include "multi_val_dense_bin.hpp"
 #include "multi_val_sparse_bin.hpp"
 #include "sparse_bin.hpp"
@@ -633,9 +632,10 @@ namespace LightGBM {
     return ret;
   }
 
-  template class DenseBin<uint8_t>;
-  template class DenseBin<uint16_t>;
-  template class DenseBin<uint32_t>;
+  template class DenseBin<uint8_t, true>;
+  template class DenseBin<uint8_t, false>;
+  template class DenseBin<uint16_t, false>;
+  template class DenseBin<uint32_t, false>;
 
   template class SparseBin<uint8_t>;
   template class SparseBin<uint16_t>;
@@ -647,13 +647,13 @@ namespace LightGBM {
 
   Bin* Bin::CreateDenseBin(data_size_t num_data, int num_bin) {
     if (num_bin <= 16) {
-      return new Dense4bitsBin(num_data);
+      return new DenseBin<uint8_t, true>(num_data);
     } else if (num_bin <= 256) {
-      return new DenseBin<uint8_t>(num_data);
+      return new DenseBin<uint8_t, false>(num_data);
     } else if (num_bin <= 65536) {
-      return new DenseBin<uint16_t>(num_data);
+      return new DenseBin<uint16_t, false>(num_data);
     } else {
-      return new DenseBin<uint32_t>(num_data);
+      return new DenseBin<uint32_t, false>(num_data);
     }
   }