Quick fix for memory leak in CPU Hist. (#5153)

Closes https://github.com/dmlc/xgboost/issues/3579 .

* Don't use map.

src/tree/updater_quantile_hist.cc

@@ -41,7 +41,6 @@ void QuantileHistMaker::Configure(const Args& args) {
   }
   pruner_->Configure(args);
   param_.UpdateAllowUnknown(args);
-  is_gmat_initialized_ = false;
 
   // initialise the split evaluator
   if (!spliteval_) {
@@ -54,12 +53,14 @@ void QuantileHistMaker::Configure(const Args& args) {
 void QuantileHistMaker::Update(HostDeviceVector<GradientPair> *gpair,
                                DMatrix *dmat,
                                const std::vector<RegTree *> &trees) {
-  if (is_gmat_initialized_ == false) {
+  if (dmat != p_last_dmat_ || is_gmat_initialized_ == false) {
     gmat_.Init(dmat, static_cast<uint32_t>(param_.max_bin));
     column_matrix_.Init(gmat_, param_.sparse_threshold);
     if (param_.enable_feature_grouping > 0) {
       gmatb_.Init(gmat_, column_matrix_, param_);
     }
+    // A proper solution is puting cut matrix in DMatrix, see:
+    // https://github.com/dmlc/xgboost/issues/5143
     is_gmat_initialized_ = true;
   }
   // rescale learning rate according to size of trees
@@ -72,12 +73,14 @@ void QuantileHistMaker::Update(HostDeviceVector<GradientPair> *gpair,
         param_,
         std::move(pruner_),
         std::unique_ptr<SplitEvaluator>(spliteval_->GetHostClone()),
-        int_constraint_));
+        int_constraint_, dmat));
   }
   for (auto tree : trees) {
     builder_->Update(gmat_, gmatb_, column_matrix_, gpair, dmat, tree);
   }
   param_.learning_rate = lr;
+
+  p_last_dmat_ = dmat;
 }
 
 bool QuantileHistMaker::UpdatePredictionCache(
@@ -508,12 +511,8 @@ void QuantileHistMaker::Builder::InitData(const GHistIndexMatrix& gmat,
       data_layout_ = kSparseData;
     }
   }
-  {
+  // store a pointer to the tree
-    // store a pointer to the tree
+  p_last_tree_ = &tree;
-    p_last_tree_ = &tree;
-    // store a pointer to training data
-    p_last_fmat_ = &fmat;
-  }
   if (data_layout_ == kDenseDataOneBased) {
     column_sampler_.Init(info.num_col_, param_.colsample_bynode, param_.colsample_bylevel,
             param_.colsample_bytree, true);

src/tree/updater_quantile_hist.h

@@ -19,6 +19,7 @@
 #include <unordered_map>
 #include <utility>
 
+#include "xgboost/data.h"
 #include "xgboost/json.h"
 #include "constraints.h"
 #include "./param.h"
@@ -80,7 +81,7 @@ using xgboost::common::Column;
 /*! \brief construct a tree using quantized feature values */
 class QuantileHistMaker: public TreeUpdater {
  public:
-  QuantileHistMaker() : is_gmat_initialized_{ false } {}
+  QuantileHistMaker() {}
   void Configure(const Args& args) override;
 
   void Update(HostDeviceVector<GradientPair>* gpair,
@@ -112,7 +113,8 @@ class QuantileHistMaker: public TreeUpdater {
   GHistIndexBlockMatrix gmatb_;
   // column accessor
   ColumnMatrix column_matrix_;
-  bool is_gmat_initialized_;
+  DMatrix const* p_last_dmat_ {nullptr};
+  bool is_gmat_initialized_ {false};
 
   // data structure
   struct NodeEntry {
@@ -136,10 +138,11 @@ class QuantileHistMaker: public TreeUpdater {
     explicit Builder(const TrainParam& param,
                      std::unique_ptr<TreeUpdater> pruner,
                      std::unique_ptr<SplitEvaluator> spliteval,
-                     FeatureInteractionConstraintHost int_constraints_)
+                     FeatureInteractionConstraintHost int_constraints_,
+                     DMatrix const* fmat)
       : param_(param), pruner_(std::move(pruner)),
         spliteval_(std::move(spliteval)), interaction_constraints_{int_constraints_},
-        p_last_tree_(nullptr), p_last_fmat_(nullptr) {
+        p_last_tree_(nullptr), p_last_fmat_(fmat) {
       builder_monitor_.Init("Quantile::Builder");
     }
     // update one tree, growing
@@ -313,7 +316,7 @@ class QuantileHistMaker: public TreeUpdater {
 
     // back pointers to tree and data matrix
     const RegTree* p_last_tree_;
-    const DMatrix* p_last_fmat_;
+    DMatrix const* const p_last_fmat_;
 
     using ExpandQueue =
        std::priority_queue<ExpandEntry, std::vector<ExpandEntry>,

tests/cpp/tree/test_quantile_hist.cc

@@ -13,6 +13,7 @@
 #include "../../../src/tree/param.h"
 #include "../../../src/tree/updater_quantile_hist.h"
 #include "../../../src/tree/split_evaluator.h"
+#include "xgboost/data.h"
 
 namespace xgboost {
 namespace tree {
@@ -26,8 +27,9 @@ class QuantileHistMock : public QuantileHistMaker {
     BuilderMock(const TrainParam& param,
                 std::unique_ptr<TreeUpdater> pruner,
                 std::unique_ptr<SplitEvaluator> spliteval,
-                FeatureInteractionConstraintHost int_constraint)
+                FeatureInteractionConstraintHost int_constraint,
-        : RealImpl(param, std::move(pruner), std::move(spliteval), std::move(int_constraint)) {}
+                DMatrix const* fmat)
+        : RealImpl(param, std::move(pruner), std::move(spliteval), std::move(int_constraint), fmat) {}
 
    public:
     void TestInitData(const GHistIndexMatrix& gmat,
@@ -236,13 +238,14 @@ class QuantileHistMock : public QuantileHistMaker {
       cfg_{args} {
     QuantileHistMaker::Configure(args);
     spliteval_->Init(&param_);
+    dmat_ = CreateDMatrix(kNRows, kNCols, 0.8, 3);
     builder_.reset(
         new BuilderMock(
             param_,
             std::move(pruner_),
             std::unique_ptr<SplitEvaluator>(spliteval_->GetHostClone()),
-            int_constraint_));
+            int_constraint_,
-    dmat_ = CreateDMatrix(kNRows, kNCols, 0.8, 3);
+            dmat_->get()));
   }
   ~QuantileHistMock() override { delete dmat_; }