From 635277d7363c5e18a72a7219c3e24fadf987833d Mon Sep 17 00:00:00 2001 From: Teebonne <80053070+Teebonne@users.noreply.github.com> Date: Fri, 9 Sep 2022 22:58:30 +0100 Subject: [PATCH] Improving memory usage and allocation (#4) * Improving memory usage and allocation (#3) * Improving memory usage and allocation (#2) * Improving memory usage and allocation No need for an optional to a pointer * Switching to a pointer directly Less memory usage and allocations * Switching to pointer, it needs to be initialized * Making value_text_ as optional to reduce memory usage Making value_text_ as optional to reduce memory usage * Making value_text_ as optional to reduce memory usage Making value_text_ as optional to reduce memory usage * Making value_text_ as optional to reduce memory usage Making value_text_ as optional to reduce memory usage * Fixing mistake when emty --- source/cell/cell.cpp | 32 +++++++++---------- source/detail/implementations/cell_impl.cpp | 4 ++- source/detail/implementations/cell_impl.hpp | 12 +++---- source/detail/serialization/xlsx_consumer.cpp | 3 +- 4 files changed, 27 insertions(+), 24 deletions(-) diff --git a/source/cell/cell.cpp b/source/cell/cell.cpp index f3d69a08..e8accdc1 100644 --- a/source/cell/cell.cpp +++ b/source/cell/cell.cpp @@ -504,8 +504,8 @@ void cell::error(const std::string &error) { throw invalid_data_type(); } - - d_->value_text_.plain_text(error, false); + d_->value_text_ = error; + d_->value_text_.get().plain_text(error, false); d_->type_ = type::error; } @@ -717,7 +717,7 @@ XLNT_API rich_text cell::value() const return workbook().shared_strings(static_cast(d_->value_numeric_)); } - return d_->value_text_; + return d_->value_text_.get(); } bool cell::has_value() const @@ -750,7 +750,7 @@ std::string cell::to_string() const bool cell::has_format() const { - return d_->format_.is_set(); + return (d_->format_ != nullptr); } void cell::format(const class format new_format) @@ -838,10 +838,10 @@ void cell::value(const std::string &value_string, bool infer_type) void cell::clear_format() { - if (d_->format_.is_set()) + if (d_->format_ != nullptr) { format().d_->references -= format().d_->references > 0 ? 1 : 0; - d_->format_.clear(); + d_->format_ = nullptr; } } @@ -899,22 +899,22 @@ bool cell::has_style() const format cell::modifiable_format() { - if (!d_->format_.is_set()) + if (d_->format_ == nullptr) { throw invalid_attribute(); } - return xlnt::format(d_->format_.get()); + return xlnt::format(d_->format_); } const format cell::format() const { - if (!d_->format_.is_set()) + if (d_->format_ == nullptr) { throw invalid_attribute(); } - return xlnt::format(d_->format_.get()); + return xlnt::format(d_->format_); } alignment cell::alignment() const @@ -956,7 +956,7 @@ bool cell::has_hyperlink() const bool cell::has_comment() { - return d_->comment_.is_set(); + return (d_->comment_ != nullptr); } void cell::clear_comment() @@ -964,7 +964,7 @@ void cell::clear_comment() if (has_comment()) { d_->parent_->comments_.erase(reference().to_string()); - d_->comment_.clear(); + d_->comment_ = nullptr; } } @@ -975,7 +975,7 @@ class comment cell::comment() throw xlnt::exception("cell has no comment"); } - return *d_->comment_.get(); + return *d_->comment_; } void cell::comment(const std::string &text, const std::string &author) @@ -992,12 +992,12 @@ void cell::comment(const class comment &new_comment) { if (has_comment()) { - *d_->comment_.get() = new_comment; + *d_->comment_ = new_comment; } else { d_->parent_->comments_[reference().to_string()] = new_comment; - d_->comment_.set(&d_->parent_->comments_[reference().to_string()]); + d_->comment_ = (&d_->parent_->comments_[reference().to_string()]); } // offset comment 5 pixels down and 5 pixels right of the top right corner of the cell @@ -1005,7 +1005,7 @@ void cell::comment(const class comment &new_comment) cell_position.first += static_cast(width()) + 5; cell_position.second += 5; - d_->comment_.get()->position(cell_position.first, cell_position.second); + d_->comment_->position(cell_position.first, cell_position.second); worksheet().register_comments_in_manifest(); } diff --git a/source/detail/implementations/cell_impl.cpp b/source/detail/implementations/cell_impl.cpp index 7eac302a..ff1435ce 100644 --- a/source/detail/implementations/cell_impl.cpp +++ b/source/detail/implementations/cell_impl.cpp @@ -36,7 +36,9 @@ cell_impl::cell_impl() row_(1), is_merged_(false), phonetics_visible_(false), - value_numeric_(0) + value_numeric_(0), + format_(), + comment_() { } diff --git a/source/detail/implementations/cell_impl.hpp b/source/detail/implementations/cell_impl.hpp index fecd543a..e52e6e17 100644 --- a/source/detail/implementations/cell_impl.hpp +++ b/source/detail/implementations/cell_impl.hpp @@ -58,17 +58,17 @@ struct cell_impl bool is_merged_; bool phonetics_visible_; - rich_text value_text_; + optional value_text_; double value_numeric_; optional formula_; optional hyperlink_; - optional format_; - optional comment_; + format_impl *format_; + comment *comment_; bool is_garbage_collectible() const { - return !(type_ != cell_type::empty || is_merged_ || phonetics_visible_ || formula_.is_set() || format_.is_set() || hyperlink_.is_set()); + return !(type_ != cell_type::empty || is_merged_ || phonetics_visible_ || formula_.is_set() || format_ != nullptr || hyperlink_.is_set()); } }; @@ -84,8 +84,8 @@ inline bool operator==(const cell_impl &lhs, const cell_impl &rhs) && float_equals(lhs.value_numeric_, rhs.value_numeric_) && lhs.formula_ == rhs.formula_ && lhs.hyperlink_ == rhs.hyperlink_ - && (lhs.format_.is_set() == rhs.format_.is_set() && (!lhs.format_.is_set() || *lhs.format_.get() == *rhs.format_.get())) - && (lhs.comment_.is_set() == rhs.comment_.is_set() && (!lhs.comment_.is_set() || *lhs.comment_.get() == *rhs.comment_.get())); + && ((lhs.format_ != nullptr) == (rhs.format_ != nullptr) && ((lhs.format_ == nullptr) || *lhs.format_ == *rhs.format_)) + && ((lhs.comment_ != nullptr) == (rhs.comment_ != nullptr) && ((lhs.comment_ == nullptr) || *lhs.comment_ == *rhs.comment_)); } } // namespace detail diff --git a/source/detail/serialization/xlsx_consumer.cpp b/source/detail/serialization/xlsx_consumer.cpp index e7dce8d1..d8f6507f 100644 --- a/source/detail/serialization/xlsx_consumer.cpp +++ b/source/detail/serialization/xlsx_consumer.cpp @@ -892,7 +892,8 @@ void xlsx_consumer::read_worksheet_sheetdata() break; } case cell::type::error: { - ws_cell_impl->value_text_.plain_text(cell.value, false); + ws_cell_impl->value_text_ = std::move(cell.value); + ws_cell_impl->value_text_.get().plain_text(cell.value, false); break; } }