Fix dmlc#3579: Fix memory leak when learing rate callback is registered

**Diagnosis** The learning rate callback function calls `XGBoosterSetParam()`
to update the learning rate. The `XGBoosterSetParam()` function in turn calls
`Learner::Configure()`, which resets and re-initializes each tree updater,
calling `FastHistMaker::Init()`. The `FastHistMaker::Init()` function in turn
re-allocates internal objects that were meant to be recycled across iterations.
Thus memory usage increases over time.

**Fix** The learning rate callback should call a new function
`XGBoosterUpdateParamInPlace()`. The new function is designed so that no object
is re-allocated.

include/xgboost/c_api.h

@@ -373,6 +373,21 @@ XGB_DLL int XGBoosterSetParam(BoosterHandle handle,
                               const char *name,
                               const char *value);
 
+/*!
+ * \brief Update a parameter "in place", i.e. in a way that the underlying
+ *        objects (Booster, Learner, TreeUpdater, etc.) are preserved.
+ *        If updating the parameter would reset or re-create any of the
+ *        underlying objects, this function will fail. This function addresses
+ *        https://github.com/dmlc/xgboost/issues/3579.
+ * \param handle handle
+ * \param name parameter name
+ * \param value value of parameter
+ * \return 0 when success, -1 when failure happens
+ */
+XGB_DLL int XGBoosterUpdateParamInPlace(BoosterHandle handle,
+                                        const char *name,
+                                        const char *value);
+
 /*!
  * \brief update the model in one round using dtrain
  * \param handle handle

include/xgboost/gbm.h

@@ -43,6 +43,16 @@ class GradientBooster {
    * \param cfg configurations on both training and model parameters.
    */
   virtual void Configure(const std::vector<std::pair<std::string, std::string> >& cfg) = 0;
+  /*!
+   * \brief Update a parameter "in place", i.e. in a way that the underlying
+   *        objects (Booster, Learner, TreeUpdater, etc.) are preserved.
+   *        If updating the parameter would reset or re-create any of the
+   *        underlying objects, this function will have no effect. This function
+   *        addresses https://github.com/dmlc/xgboost/issues/3579.
+   * \param name parameter name
+   * \param value value of parameter
+   */
+  virtual void UpdateParamInPlace(const std::string& name, const std::string& value) = 0;
   /*!
    * \brief load model from stream
    * \param fi input stream.

include/xgboost/learner.h

@@ -53,6 +53,16 @@ class Learner : public rabit::Serializable {
    * \param cfg configurations on both training and model parameters.
    */
   virtual void Configure(const std::vector<std::pair<std::string, std::string> >& cfg) = 0;
+  /*!
+   * \brief Update a parameter "in place", i.e. in a way that the underlying
+   *        objects (Booster, Learner, TreeUpdater, etc.) are preserved.
+   *        If updating the parameter would reset or re-create any of the
+   *        underlying objects, this function will have no effect. This function
+   *        addresses https://github.com/dmlc/xgboost/issues/3579.
+   * \param name parameter name
+   * \param value value of parameter
+   */
+  virtual void UpdateParamInPlace(const std::string& name, const std::string& value) = 0;
   /*!
    * \brief Initialize the model using the specified configurations via Configure.
    *  An model have to be either Loaded or initialized before Update/Predict/Save can be called.

include/xgboost/linear_updater.h

@@ -12,6 +12,7 @@
 #include <vector>
 #include "../../src/gbm/gblinear_model.h"
 #include "../../src/common/host_device_vector.h"
+#include "../../src/common/common.h"
 
 namespace xgboost {
 /*!
@@ -28,6 +29,17 @@ class LinearUpdater {
   virtual void Init(
       const std::vector<std::pair<std::string, std::string> >& args) = 0;
 
+  /*!
+   * \brief Update a parameter "in place", i.e. in a way that the underlying
+   *        objects (Booster, Learner, TreeUpdater, etc.) are preserved.
+   *        If updating the parameter would reset or re-create any of the
+   *        underlying objects, this function will have no effect. This function
+   *        addresses https://github.com/dmlc/xgboost/issues/3579.
+   * \param name parameter name
+   * \param value value of parameter
+   */
+  virtual void UpdateParamInPlace(const std::string& name, const std::string& value) = 0;
+
   /**
    * \brief Updates linear model given gradients.
    *

include/xgboost/objective.h

@@ -37,6 +37,16 @@ class ObjFunction {
    * \param args arguments to the objective function.
    */
   virtual void Configure(const std::vector<std::pair<std::string, std::string> >& args) = 0;
+  /*!
+   * \brief Update a parameter "in place", i.e. in a way that the underlying
+   *        objects (Booster, Learner, TreeUpdater, etc.) are preserved.
+   *        If updating the parameter would reset or re-create any of the
+   *        underlying objects, this function will have no effect. This function
+   *        addresses https://github.com/dmlc/xgboost/issues/3579.
+   * \param name parameter name
+   * \param value value of parameter
+   */
+  virtual void UpdateParamInPlace(const std::string& name, const std::string& value) {}
   /*!
    * \brief Get gradient over each of predictions, given existing information.
    * \param preds prediction of current round

include/xgboost/tree_updater.h

@@ -17,6 +17,7 @@
 #include "./data.h"
 #include "./tree_model.h"
 #include "../../src/common/host_device_vector.h"
+#include "../../src/common/common.h"
 
 namespace xgboost {
 /*!
@@ -43,6 +44,16 @@ class TreeUpdater {
   virtual void Update(HostDeviceVector<GradientPair>* gpair,
                       DMatrix* data,
                       const std::vector<RegTree*>& trees) = 0;
+  /*!
+   * \brief Update a parameter "in place", i.e. in a way that the underlying
+   *        objects (Booster, Learner, TreeUpdater, etc.) are preserved.
+   *        If updating the parameter would reset or re-create any of the
+   *        underlying objects, this function will have no effect. This function
+   *        addresses https://github.com/dmlc/xgboost/issues/3579.
+   * \param name parameter name
+   * \param value value of parameter
+   */
+  virtual void UpdateParamInPlace(const std::string& name, const std::string& value) = 0;
 
   /*!
    * \brief determines whether updater has enough knowledge about a given dataset

python-package/xgboost/callback.py

@@ -136,12 +136,14 @@ def callback(env):
 
         if context == 'train':
             bst, i, n = env.model, env.iteration, env.end_iteration
-            bst.set_param('learning_rate', get_learning_rate(i, n, learning_rates))
+            bst.update_param_in_place('learning_rate',
+                                      get_learning_rate(i, n, learning_rates))
         elif context == 'cv':
             i, n = env.iteration, env.end_iteration
             for cvpack in env.cvfolds:
                 bst = cvpack.bst
-                bst.set_param('learning_rate', get_learning_rate(i, n, learning_rates))
+                bst.update_param_in_place('learning_rate',
+                                          get_learning_rate(i, n, learning_rates))
 
     callback.before_iteration = True
     return callback

python-package/xgboost/core.py

@@ -1023,6 +1023,22 @@ def set_param(self, params, value=None):
         for key, val in params:
             _check_call(_LIB.XGBoosterSetParam(self.handle, c_str(key), c_str(str(val))))
 
+    def update_param_in_place(self, param, value):
+        """Update a parameter "in place", i.e. in a way that the underlying
+           objects (Booster, Learner, TreeUpdater, etc.) are preserved.
+           If updating the parameter would reset or re-create any of the
+           underlying objects, this function will fail. This function addresses
+           https://github.com/dmlc/xgboost/issues/3579.
+
+        Parameters
+        ----------
+        param: str
+           key of parameter to update
+        value: optional
+           value of the specified parameter
+        """
+        _check_call(_LIB.XGBoosterUpdateParamInPlace(self.handle, c_str(param), c_str(str(value))))
+
     def update(self, dtrain, iteration, fobj=None):
         """
         Update for one iteration, with objective function calculated internally.

src/c_api/c_api.cc

@@ -7,6 +7,7 @@
 #include <dmlc/thread_local.h>
 #include <rabit/rabit.h>
 #include <cstdio>
+#include <unordered_set>
 #include <vector>
 #include <string>
 #include <cstring>
@@ -50,6 +51,13 @@ class Booster {
     }
   }
 
+  inline void UpdateParamInPlace(const std::string& name,
+                                 const std::string& val) {
+    CHECK_NE(name, "eval_metric")
+      << "Booster::UpdateParamInPlace(): eval_metric cannot be updated in-place";
+    learner_->UpdateParamInPlace(name, val);
+  }
+
   inline void LazyInit() {
     if (!configured_) {
       learner_->Configure(cfg_);
@@ -870,6 +878,19 @@ XGB_DLL int XGBoosterSetParam(BoosterHandle handle,
   API_END();
 }
 
+XGB_DLL int XGBoosterUpdateParamInPlace(BoosterHandle handle,
+                                        const char *name,
+                                        const char *value) {
+  API_BEGIN();
+  CHECK_HANDLE();
+  // List of parameters allowed for in-place update
+  const std::unordered_set<std::string> whitelist{"learning_rate", "eta"};
+  CHECK_GT(whitelist.count(name), 0)
+    << "XGBoosterUpdateParamInPlace(): Parameter " << name << " is not allowed";
+  static_cast<Booster*>(handle)->UpdateParamInPlace(name, value);
+  API_END();
+}
+
 XGB_DLL int XGBoosterUpdateOneIter(BoosterHandle handle,
                                    int iter,
                                    DMatrixHandle dtrain) {

src/common/common.h

@@ -8,6 +8,7 @@
 
 #include <xgboost/base.h>
 #include <xgboost/logging.h>
+#include <dmlc/parameter.h>
 
 #include <exception>
 #include <limits>
@@ -64,6 +65,26 @@ inline std::vector<std::string> Split(const std::string& s, char delim) {
   return ret;
 }
 
+/*!
+ * \brief Default implementation of UpdateParamInPlace for subclasses of
+ *        dmlc::Parameter
+ * \tparam ParamType subclass of dmlc::Parameter
+ * \param param object of a subclass of dmlc::Parameter
+ * \param name parameter name
+ * \param value value of parameter
+ */
+template <typename ParamType>
+inline void UpdateParamInPlaceDefault(ParamType* param, const std::string& name,
+                                      const std::string& value) {
+  CHECK(param);
+  dmlc::parameter::ParamManager* manager = ParamType::__MANAGER__();
+  CHECK(manager);
+  dmlc::parameter::FieldAccessEntry* entry = manager->Find(name);
+  if (entry) {
+    entry->Set(param, value);
+  }
+}
+
 // simple routine to convert any data to string
 template<typename T>
 inline std::string ToString(const T& data) {

src/gbm/gblinear.cc

@@ -71,6 +71,9 @@ class GBLinear : public GradientBooster {
     updater_->Init(cfg);
     monitor_.Init("GBLinear ", param_.debug_verbose);
   }
+  void UpdateParamInPlace(const std::string& name, const std::string& value) override {
+    updater_->UpdateParamInPlace(name, value);
+  }
   void Load(dmlc::Stream* fi) override {
     model_.Load(fi);
   }

src/gbm/gbtree.cc

@@ -163,6 +163,12 @@ class GBTree : public GradientBooster {
     monitor_.Init("GBTree", tparam_.debug_verbose);
   }
 
+  void UpdateParamInPlace(const std::string& name, const std::string& value) override {
+    for (const auto& up : updaters_) {
+      up->UpdateParamInPlace(name, value);
+    }
+  }
+
   void Load(dmlc::Stream* fi) override {
     model_.Load(fi);
 

src/gbm/gbtree_model.h

@@ -1,7 +1,9 @@
 /*!
  * Copyright by Contributors 2017
  */
-#pragma once
+#ifndef XGBOOST_GBM_GBTREE_MODEL_H_
+#define XGBOOST_GBM_GBTREE_MODEL_H_
+
 #include <dmlc/parameter.h>
 #include <dmlc/io.h>
 #include <xgboost/tree_model.h>
@@ -138,3 +140,4 @@ struct GBTreeModel {
 };
 }  // namespace gbm
 }  // namespace xgboost
+#endif  // XGBOOST_GBM_GBTREE_MODEL_H_

src/learner.cc

@@ -259,6 +259,16 @@ class LearnerImpl : public Learner {
     }
   }
 
+  void UpdateParamInPlace(const std::string& name,
+                          const std::string& value) override {
+    if (gbm_ != nullptr) {
+      gbm_->UpdateParamInPlace(name, value);
+    }
+    if (obj_ != nullptr) {
+      obj_->UpdateParamInPlace(name, value);
+    }
+  }
+
   void InitModel() override { this->LazyInitModel(); }
 
   void Load(dmlc::Stream* fi) override {

src/linear/updater_coordinate.cc

@@ -84,6 +84,9 @@ class CoordinateUpdater : public LinearUpdater {
     selector.reset(FeatureSelector::Create(param.feature_selector));
     monitor.Init("CoordinateUpdater", param.debug_verbose);
   }
+  void UpdateParamInPlace(const std::string& name, const std::string& value) override {
+    common::UpdateParamInPlaceDefault(&param, name, value);
+  }
   void Update(HostDeviceVector<GradientPair> *in_gpair, DMatrix *p_fmat,
               gbm::GBLinearModel *model, double sum_instance_weight) override {
     param.DenormalizePenalties(sum_instance_weight);

src/linear/updater_gpu_coordinate.cu

@@ -212,6 +212,10 @@ class GPUCoordinateUpdater : public LinearUpdater {
     monitor.Init("GPUCoordinateUpdater", param.debug_verbose);
   }
 
+  void UpdateParamInPlace(const std::string& name, const std::string& value) override {
+    common::UpdateParamInPlaceDefault(&param, name, value);
+  }
+
   void LazyInitShards(DMatrix *p_fmat,
                       const gbm::GBLinearModelParam &model_param) {
     if (!shards.empty()) return;

src/linear/updater_shotgun.cc

@@ -61,6 +61,9 @@ class ShotgunUpdater : public LinearUpdater {
     param_.InitAllowUnknown(args);
     selector_.reset(FeatureSelector::Create(param_.feature_selector));
   }
+  void UpdateParamInPlace(const std::string& name, const std::string& value) override {
+    common::UpdateParamInPlaceDefault(&param_, name, value);
+  }
   void Update(HostDeviceVector<GradientPair> *in_gpair, DMatrix *p_fmat,
               gbm::GBLinearModel *model, double sum_instance_weight) override {
     auto &gpair = in_gpair->HostVector();

src/tree/updater_basemaker-inl.h

@@ -32,6 +32,10 @@ class BaseMaker: public TreeUpdater {
     param_.InitAllowUnknown(args);
   }
 
+  void UpdateParamInPlace(const std::string& name, const std::string& value) override {
+    common::UpdateParamInPlaceDefault(&param_, name, value);
+  }
+
  protected:
   // helper to collect and query feature meta information
   struct FMetaHelper {

src/tree/updater_colmaker.cc

@@ -29,6 +29,10 @@ class ColMaker: public TreeUpdater {
     spliteval_->Init(args);
   }
 
+  void UpdateParamInPlace(const std::string& name, const std::string& value) override {
+    common::UpdateParamInPlaceDefault(&param_, name, value);
+  }
+
   void Update(HostDeviceVector<GradientPair> *gpair,
               DMatrix* dmat,
               const std::vector<RegTree*> &trees) override {
@@ -763,6 +767,9 @@ class DistColMaker : public ColMaker {
     spliteval_.reset(SplitEvaluator::Create(param_.split_evaluator));
     spliteval_->Init(args);
   }
+  void UpdateParamInPlace(const std::string& name, const std::string& value) override {
+    common::UpdateParamInPlaceDefault(&param_, name, value);
+  }
   void Update(HostDeviceVector<GradientPair> *gpair,
               DMatrix* dmat,
               const std::vector<RegTree*> &trees) override {

src/tree/updater_fast_hist.cc

@@ -63,6 +63,11 @@ class FastHistMaker: public TreeUpdater {
     spliteval_->Init(args);
   }
 
+  void UpdateParamInPlace(const std::string& name, const std::string& value) override {
+    common::UpdateParamInPlaceDefault(&param_, name, value);
+    common::UpdateParamInPlaceDefault(&fhparam_, name, value);
+  }
+
   void Update(HostDeviceVector<GradientPair>* gpair,
               DMatrix* dmat,
               const std::vector<RegTree*>& trees) override {

src/tree/updater_gpu.cu

@@ -520,6 +520,10 @@ class GPUMaker : public TreeUpdater {
     devices_ = GPUSet::All(param.n_gpus).Normalised(param.gpu_id);
   }
 
+  void UpdateParamInPlace(const std::string& name, const std::string& value) override {
+    common::UpdateParamInPlaceDefault(&param, name, value);
+  }
+
   void Update(HostDeviceVector<GradientPair>* gpair, DMatrix* dmat,
               const std::vector<RegTree*>& trees) override {
     GradStats::CheckInfo(dmat->Info());

src/tree/updater_gpu_hist.cu

@@ -770,6 +770,10 @@ class GPUHistMaker : public TreeUpdater {
     monitor_.Init("updater_gpu_hist", param_.debug_verbose);
   }
 
+  void UpdateParamInPlace(const std::string& name, const std::string& value) override {
+    common::UpdateParamInPlaceDefault(&param_, name, value);
+  }
+
   void Update(HostDeviceVector<GradientPair>* gpair, DMatrix* dmat,
               const std::vector<RegTree*>& trees) override {
     monitor_.Start("Update", dist_.Devices());