Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quick fix for memory leak in CPU Hist. #5153

Merged
merged 2 commits into from
Dec 31, 2019
Merged

Quick fix for memory leak in CPU Hist. #5153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b991993
Quick fix for memory leak.
trivialfis Dec 23, 2019
746d058
Don't use map.
trivialfis Dec 24, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 8 additions & 9 deletions src/tree/updater_quantile_hist.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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_) {
Expand All @@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -508,12 +511,8 @@ void QuantileHistMaker::Builder::InitData(const GHistIndexMatrix& gmat,
data_layout_ = kSparseData;
}
}
{
// store a pointer to the tree
p_last_tree_ = &tree;
// store a pointer to training data
p_last_fmat_ = &fmat;
}
// store a pointer to the tree
p_last_tree_ = &tree;
if (data_layout_ == kDenseDataOneBased) {
column_sampler_.Init(info.num_col_, param_.colsample_bynode, param_.colsample_bylevel,
param_.colsample_bytree, true);
Expand Down
13 changes: 8 additions & 5 deletions src/tree/updater_quantile_hist.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <unordered_map>
#include <utility>

#include "xgboost/data.h"
#include "xgboost/json.h"
#include "constraints.h"
#include "./param.h"
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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>,
Expand Down
11 changes: 7 additions & 4 deletions tests/cpp/tree/test_quantile_hist.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
: RealImpl(param, std::move(pruner), std::move(spliteval), std::move(int_constraint)) {}
FeatureInteractionConstraintHost int_constraint,
DMatrix const* fmat)
: RealImpl(param, std::move(pruner), std::move(spliteval), std::move(int_constraint), fmat) {}

public:
void TestInitData(const GHistIndexMatrix& gmat,
Expand Down Expand Up @@ -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_));
dmat_ = CreateDMatrix(kNRows, kNCols, 0.8, 3);
int_constraint_,
dmat_->get()));
}
~QuantileHistMock() override { delete dmat_; }

Expand Down