From 198f3a6c4a1f9b315395e8c3fbee5d829199b425 Mon Sep 17 00:00:00 2001 From: sriramch <33358417+sriramch@users.noreply.github.com> Date: Fri, 9 Aug 2019 08:47:35 -0700 Subject: [PATCH] Enable natural copies of the batch iterators without the need of the clone method (#4748) - the synthesized copy constructor should do the appropriate job --- include/xgboost/data.h | 11 +---------- src/data/simple_dmatrix.cc | 3 --- src/data/sparse_page_dmatrix.cc | 3 --- 3 files changed, 1 insertion(+), 16 deletions(-) diff --git a/include/xgboost/data.h b/include/xgboost/data.h index 6cae56349..50134c13b 100644 --- a/include/xgboost/data.h +++ b/include/xgboost/data.h @@ -285,7 +285,6 @@ template class BatchIteratorImpl { public: virtual ~BatchIteratorImpl() {} - virtual BatchIteratorImpl* Clone() = 0; virtual T& operator*() = 0; virtual const T& operator*() const = 0; virtual void operator++() = 0; @@ -298,14 +297,6 @@ class BatchIterator { using iterator_category = std::forward_iterator_tag; explicit BatchIterator(BatchIteratorImpl* impl) { impl_.reset(impl); } - BatchIterator(const BatchIterator& other) { - if (other.impl_) { - impl_.reset(other.impl_->Clone()); - } else { - impl_.reset(); - } - } - void operator++() { CHECK(impl_ != nullptr); ++(*impl_); @@ -332,7 +323,7 @@ class BatchIterator { } private: - std::unique_ptr> impl_; + std::shared_ptr> impl_; }; template diff --git a/src/data/simple_dmatrix.cc b/src/data/simple_dmatrix.cc index fbe1a1edf..8fb6e2d97 100644 --- a/src/data/simple_dmatrix.cc +++ b/src/data/simple_dmatrix.cc @@ -43,9 +43,6 @@ class SimpleBatchIteratorImpl : public BatchIteratorImpl { } void operator++() override { page_ = nullptr; } bool AtEnd() const override { return page_ == nullptr; } - SimpleBatchIteratorImpl* Clone() override { - return new SimpleBatchIteratorImpl(*this); - } private: T* page_{nullptr}; diff --git a/src/data/sparse_page_dmatrix.cc b/src/data/sparse_page_dmatrix.cc index a755927ba..a1c73f1e6 100644 --- a/src/data/sparse_page_dmatrix.cc +++ b/src/data/sparse_page_dmatrix.cc @@ -31,9 +31,6 @@ class SparseBatchIteratorImpl : public BatchIteratorImpl { const T& operator*() const override { return source_->Value(); } void operator++() override { at_end_ = !source_->Next(); } bool AtEnd() const override { return at_end_; } - SparseBatchIteratorImpl* Clone() override { - return new SparseBatchIteratorImpl(*this); - } private: SparsePageSource* source_{nullptr};