From 52137e3eb159460d55b3deaae30641fa7cda9750 Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Thu, 22 Sep 2022 01:36:53 +0000 Subject: [PATCH] Use malloc instead of std::vector To improve the performance of the parcel creation, the data_ variable of the parcel implementation is changed to uint8_t*. Adds: - Parcel.GetData() - Parcel.GetDataSize() - Parcel.GetDataCapacity() - Parcel.SetDataCapacity() - Parcel.Detach() - Parcel.GetReader() - Parcel.ToRaw() - parcel_get_data_capacity() - parcel_set_data_capacity() Removes: - Parcel.GetRaw() Change-Id: Ia88815746d646ce089de802d89017c96cbc6b30d Signed-off-by: Hwankyu Jhun --- parcel/api/parcel.h | 26 ++++ parcel/parcel.cc | 226 ++++++++++++++++++++++++------ parcel/parcel.hh | 65 +++++++-- parcel/parcel_implementation.hh | 20 ++- parcel/stub.cc | 38 ++++- tests/parcel_unittests/test_parcel.cc | 27 ++++ tests/parcel_unittests/test_parcel_cpp.cc | 53 ++++++- 7 files changed, 388 insertions(+), 67 deletions(-) diff --git a/parcel/api/parcel.h b/parcel/api/parcel.h index 640b3b1..10dbaba 100644 --- a/parcel/api/parcel.h +++ b/parcel/api/parcel.h @@ -529,6 +529,32 @@ int parcel_get_raw(parcel_h parcel, void **raw, uint32_t *size); */ int parcel_set_byte_order(parcel_h parcle, bool big_endian); +/** + * @brief Gets the size of the data capacity of the parcel handle. + * @since_tizen 7.0 + * @param[in] parcel The parcel handle + * @param[out] size The size of the data capacity + * @return @c 0 on success, + * otherwise a negative error value + * @retval #PARCEL_ERROR_NONE Successful + * @retval #PARCEL_ERROR_INVALID_PARAMETER Invalid parameter + */ +int parcel_get_data_capacity(parcel_h parcel, size_t *size); + +/** + * @brief Sets the size of the data capacity of the parcel handle. + * @since_tizen 7.0 + * @remarks The raw data will be reallocated using the given size. + * @param[in] parcel The parcel handle + * @param[in] size The size of the data capacity + * @return @c 0 on success, + * otherwise a negative error value + * @retval #PARCEL_ERROR_NONE Successful + * @retval #PARCEL_ERROR_INVALID_PARAMETER Invalid parameter + * @retval #PARCEL_ERROR_OUT_OF_MEMORY + */ +int parcel_set_data_capacity(parcel_h parcel, size_t size); + #ifdef __cplusplus } #endif diff --git a/parcel/parcel.cc b/parcel/parcel.cc index 705fa9b..439e212 100644 --- a/parcel/parcel.cc +++ b/parcel/parcel.cc @@ -16,6 +16,9 @@ #include +#include +#include + #include "parcel/common.hh" #include "parcel/log_private.hh" #include "parcel/parcel.hh" @@ -23,42 +26,58 @@ #include "parcel/parcelable.hh" namespace tizen_base { +namespace { + +constexpr const size_t kDataCapacity = 128; + +} // namespace -Parcel::Impl::Impl(Parcel* parent, bool big_endian) - : parent_(parent), big_endian_(big_endian) { +Parcel::Impl::Impl(Parcel* parent, size_t data_capacity, uint8_t* data) + : parent_(parent), data_capacity_(data_capacity), data_(data) { + if (data_ != nullptr) + data_size_ = data_capacity_; + else + data_ = static_cast(malloc(data_capacity_)); } -Parcel::Impl::~Impl() {} +Parcel::Impl::~Impl() { + free(data_); +} void Parcel::Impl::Write(const void* buf, uint32_t size) { - auto* p = reinterpret_cast(buf); - data_.insert(data_.end(), p, p + size); + if (data_size_ + size > data_capacity_) { + size_t new_size = (data_capacity_ + size) * 3 / 2; + uint8_t* data = static_cast(realloc(data_, new_size)); + if (data == nullptr) + throw std::bad_alloc(); + + data_ = data; + data_capacity_ = new_size; + } + + memcpy(data_ + data_size_, buf, size); + data_size_ += size; } int Parcel::Impl::Read(void* buf, uint32_t size) { - if (data_.size() == 0) + if (data_size_ == 0) return TIZEN_ERROR_NO_DATA; - if (reader_ + size > data_.size()) + if (reader_ + size > data_size_) return TIZEN_ERROR_ILLEGAL_BYTE_SEQ; - auto* p = reinterpret_cast(buf); - std::copy(&data_[reader_], &data_[reader_] + size, p); + memcpy(buf, data_ + reader_, size); reader_ += size; set_last_result(TIZEN_ERROR_NONE); return TIZEN_ERROR_NONE; } -const std::vector& Parcel::Impl::GetRaw() { - return data_; -} - void Parcel::Impl::ResetReader() { reader_ = 0; } void Parcel::Impl::Clear() { - data_.clear(); + data_size_ = 0; reader_ = 0; } @@ -68,7 +87,7 @@ void Parcel::Impl::Reset(const void* buf, uint32_t size) { } bool Parcel::Impl::IsEmpty() { - return data_.size() == 0; + return data_size_ == 0; } void Parcel::Impl::WriteSize(uint32_t size) { @@ -80,20 +99,28 @@ void Parcel::Impl::WriteSize(uint32_t size) { template void Parcel::Impl::Write(T d) { + if (data_size_ + sizeof(T) > data_capacity_) { + size_t new_size = (data_capacity_ + sizeof(T)) * 3 / 2; + uint8_t* data = static_cast(realloc(data_, new_size)); + if (data == nullptr) + throw std::bad_alloc(); + + data_ = data; + data_capacity_ = new_size; + } + auto* p = reinterpret_cast(&d); - data_.insert(data_.end(), p, p + sizeof(T)); + memcpy(data_ + data_size_, p, sizeof(T)); + data_size_ += sizeof(T); } int Parcel::Impl::ReadSize(uint32_t* size) { - if (data_.size() == 0) + if (data_size_ == 0) return TIZEN_ERROR_NO_DATA; - if (reader_ + sizeof(uint32_t) > data_.size()) - return TIZEN_ERROR_ILLEGAL_BYTE_SEQ; - - auto* p = reinterpret_cast(size); - std::copy(&data_[reader_], &data_[reader_] + sizeof(uint32_t), p); - reader_ += sizeof(uint32_t); + int ret = Read(size); + if (ret != TIZEN_ERROR_NONE) + return ret; if (IsBigEndian()) *size = GUINT32_FROM_BE(*size); @@ -104,11 +131,11 @@ int Parcel::Impl::ReadSize(uint32_t* size) { template int Parcel::Impl::Read(T* d) { uint32_t size = sizeof(T); - if (reader_ + size > data_.size()) + if (reader_ + size > data_size_) return TIZEN_ERROR_ILLEGAL_BYTE_SEQ; auto* p = reinterpret_cast(d); - std::copy(&data_[reader_], &data_[reader_] + size, p); + memcpy(p, data_ + reader_, size); reader_ += size; return TIZEN_ERROR_NONE; } @@ -117,46 +144,139 @@ void Parcel::Impl::SetByteOrder(bool big_endian) { big_endian_ = big_endian; } -bool Parcel::Impl::IsBigEndian() { +bool Parcel::Impl::IsBigEndian() const { return big_endian_; } -Parcel::Parcel(bool big_endian) : impl_(new Impl(this, big_endian)) { +uint8_t* Parcel::Impl::GetData() const { + return data_; } -Parcel::Parcel(const void* buf, uint32_t size, bool big_endian) - : impl_(new Impl(this, big_endian)) { - impl_->Write(buf, size); +size_t Parcel::Impl::GetDataSize() const { + return data_size_; +} + +size_t Parcel::Impl::GetDataCapacity() const { + return data_capacity_; +} + +void Parcel::Impl::SetDataCapacity(size_t size) { + uint8_t* data = static_cast(realloc(data_, size)); + if (data == nullptr) + throw std::bad_alloc(); + + data_ = data; + data_capacity_ = size; +} + +uint8_t* Parcel::Impl::Detach(size_t* size) { + uint8_t* data = data_; + *size = data_size_; + data_ = nullptr; + data_capacity_ = 0; + data_size_ = 0; + reader_= 0; + return data; +} + +uint32_t Parcel::Impl::GetReader() const { + return reader_; +} + +std::vector Parcel::Impl::ToRaw() { + return std::vector(data_, data_ + data_size_); +} + +Parcel::Parcel() + : impl_(new Impl(this, kDataCapacity, nullptr)) { + if (impl_->data_ == nullptr) + throw std::bad_alloc(); +} + +Parcel::Parcel(const void* buf, uint32_t size, bool copy) + : impl_(new Impl(this, size, + copy ? nullptr : static_cast(const_cast(buf)))) { + if (impl_->data_ == nullptr) + throw std::bad_alloc(); + + if (copy) + impl_->Write(buf, size); } Parcel::~Parcel() {}; Parcel::Parcel(const Parcel& p) - : impl_(new Impl(this)) { + : impl_(new Impl(this, kDataCapacity)) { impl_->big_endian_ = p.impl_->big_endian_; - impl_->data_ = p.impl_->data_; + impl_->data_capacity_ = p.impl_->data_capacity_; + impl_->data_size_ = p.impl_->data_size_; + + uint8_t* data = static_cast( + realloc(impl_->data_, impl_->data_capacity_)); + if (data == nullptr) + throw std::bad_alloc(); + + impl_->data_ = data; + memcpy(impl_->data_, p.impl_->data_, impl_->data_size_); + impl_->reader_ = p.impl_->reader_; } Parcel& Parcel::operator = (const Parcel& p) { if (this != &p) { impl_->big_endian_ = p.impl_->big_endian_; - impl_->data_ = p.impl_->data_; + impl_->data_capacity_ = p.impl_->data_capacity_; + impl_->data_size_ = p.impl_->data_size_; + + uint8_t* data = static_cast( + realloc(impl_->data_, impl_->data_capacity_)); + if (data == nullptr) + throw std::bad_alloc(); + + impl_->data_ = data; + memcpy(impl_->data_, p.impl_->data_, impl_->data_size_); + impl_->reader_ = p.impl_->reader_; } return *this; } Parcel::Parcel(Parcel&& p) noexcept - : impl_(new Impl(this)) { - impl_ = std::move(p.impl_); - impl_->parent_ = this; + : impl_(new Impl(this, kDataCapacity)) { + impl_->big_endian_ = p.impl_->big_endian_; + + impl_->data_size_ = p.impl_->data_size_; + p.impl_->data_size_ = 0; + + uint8_t* data = impl_->data_; + size_t data_capacity = impl_->data_capacity_; + + impl_->data_ = p.impl_->data_; + impl_->data_capacity_ = p.impl_->data_capacity_; + + p.impl_->data_ = data; + p.impl_->data_capacity_ = data_capacity; + + impl_->reader_ = p.impl_->reader_; + p.impl_->reader_ = 0; } Parcel& Parcel::operator = (Parcel&& p) noexcept { if (this != &p) { impl_->big_endian_ = p.impl_->big_endian_; - impl_->data_ = std::move(p.impl_->data_); + + impl_->data_size_ = p.impl_->data_size_; + p.impl_->data_size_ = 0; + + uint8_t* data = impl_->data_; + size_t data_capacity = impl_->data_capacity_; + + impl_->data_ = p.impl_->data_; + impl_->data_capacity_ = p.impl_->data_capacity_; + + p.impl_->data_ = data; + p.impl_->data_capacity_ = data_capacity; + impl_->reader_ = p.impl_->reader_; p.impl_->reader_ = 0; } @@ -366,10 +486,6 @@ int Parcel::ReadCString(char** str) { return TIZEN_ERROR_NONE; } -const std::vector& Parcel::GetRaw() { - return impl_->GetRaw(); -} - void Parcel::ResetReader() { impl_->ResetReader(); } @@ -395,4 +511,32 @@ void Parcel::SetByteOrder(bool big_endian) { impl_->SetByteOrder(big_endian); } +uint8_t* Parcel::GetData() const { + return impl_->GetData(); +} + +size_t Parcel::GetDataSize() const { + return impl_->GetDataSize(); +} + +size_t Parcel::GetDataCapacity() const { + return impl_->GetDataCapacity(); +} + +void Parcel::SetDataCapacity(size_t size) { + impl_->SetDataCapacity(size); +} + +uint8_t* Parcel::Detach(size_t* size) { + return impl_->Detach(size); +} + +uint32_t Parcel::GetReader() const { + return impl_->GetReader(); +} + +std::vector Parcel::ToRaw() { + return impl_->ToRaw(); +} + } // namespace tizen_base diff --git a/parcel/parcel.hh b/parcel/parcel.hh index 87af7d7..7d94d00 100644 --- a/parcel/parcel.hh +++ b/parcel/parcel.hh @@ -54,18 +54,18 @@ class EXPORT Parcel final { /** * @brief Constructor. * @since_tizen 6.5 - * @param[in] big_endian @c true, if byte order of the parcel is big-endian */ - Parcel(bool big_endian = false); + Parcel(); /** * @brief Constructor * @since_tizen 6.5 * @param[in] buf The bytes to write * @param[in] size the size of bytes to write - * @param[in] big_endian @c true, if byte order of the parcel is big-endian + * @param[in] copy If @c is true, this object copies bytes. + * If @c is false, this object takes ownership of @buf. */ - Parcel(const void* buf, uint32_t size, bool big_endian = false); + Parcel(const void* buf, uint32_t size, bool copy = true); /** * @brief Destructor @@ -329,13 +329,6 @@ class EXPORT Parcel final { int ReadCString(char** str); /** - * @brief Gets the raw data of the parcel. - * @since_tizen 6.5 - * @return The raw data - */ - const std::vector& GetRaw(); - - /** * @brief Resets the reader pointer of the parcel to the start. * @since_tizen 6.5 */ @@ -378,6 +371,56 @@ class EXPORT Parcel final { */ void SetByteOrder(bool big_endian); + /** + * @brief Gets the raw data of the parcel. + * @since_tizen 7.0 + * @return The raw data + */ + uint8_t* GetData() const; + + /** + * @breif Gets the size of the raw data of the parcel. + * @since_tizen 7.0 + * @return The size of the raw data + */ + size_t GetDataSize() const; + + /** + * @brief Gets the size of the data capacity of the parcel. + * @since_tizen 7.0 + * @return The size of the data capacity + */ + size_t GetDataCapacity() const; + + /** + * @breif Sets the size of the data capacity of the parcel. + * @since_tizen 7.0 + * @param[in] size The size of the data capcity + */ + void SetDataCapacity(size_t size); + + /** + * @brief Detaches the raw data from the parcel. + * @since_tizen 7.0 + * @param[out] size The size of the raw data + * @return The raw data + */ + uint8_t* Detach(size_t* size); + + /** + * @brief Gets the position of the reader of the parcel. + * @since_tizen 7.0 + * @return The position of the reader + */ + uint32_t GetReader() const; + + /** + * @brief Converts the raw data of the parcel to the vector. + * @since_tizen 7.0 + * @return The vector of the raw data + */ + std::vector ToRaw(); + private: class Impl; std::unique_ptr impl_; diff --git a/parcel/parcel_implementation.hh b/parcel/parcel_implementation.hh index ed6c62f..d9a9c7d 100644 --- a/parcel/parcel_implementation.hh +++ b/parcel/parcel_implementation.hh @@ -31,7 +31,6 @@ class Parcel::Impl { void Write(const void* buf, uint32_t size); int Read(void* buf, uint32_t size); - const std::vector& GetRaw(); void ResetReader(); void Clear(); void Reset(const void* buf, uint32_t size); @@ -45,17 +44,26 @@ class Parcel::Impl { int Read(T* d); void SetByteOrder(bool big_endian); - bool IsBigEndian(); + bool IsBigEndian() const; + uint8_t* GetData() const; + size_t GetDataSize() const; + size_t GetDataCapacity() const; + void SetDataCapacity(size_t size); + uint8_t* Detach(size_t* size); + uint32_t GetReader() const; + std::vector ToRaw(); private: friend class Parcel; - explicit Impl(Parcel* parent, bool big_endian = false); + explicit Impl(Parcel* parent, size_t data_capacity, uint8_t* data = nullptr); private: Parcel* parent_; - bool big_endian_; - std::vector data_; - uint64_t reader_ = 0; + size_t data_capacity_; + bool big_endian_ = false; + size_t data_size_ = 0; + uint8_t* data_ = nullptr; + uint32_t reader_ = 0; }; } // naemspace tizen_base diff --git a/parcel/stub.cc b/parcel/stub.cc index ce62584..f6b6299 100644 --- a/parcel/stub.cc +++ b/parcel/stub.cc @@ -15,6 +15,9 @@ */ #include "parcel/api/parcel.h" + +#include + #include "parcel/common.hh" #include "parcel/log_private.hh" #include "parcel/parcel.hh" @@ -386,9 +389,8 @@ extern "C" EXPORT int parcel_get_raw(parcel_h parcel, void** raw, } auto* h = static_cast(parcel); - auto& raw_data = h->GetRaw(); - *raw = reinterpret_cast(const_cast(&raw_data[0])); - *size = raw_data.size(); + *raw = reinterpret_cast(h->GetData()); + *size = h->GetDataSize(); return PARCEL_ERROR_NONE; } @@ -403,3 +405,33 @@ extern "C" EXPORT int parcel_set_byte_order(parcel_h parcel, h->SetByteOrder(big_endian); return PARCEL_ERROR_NONE; } + +extern "C" EXPORT int parcel_get_data_capacity(parcel_h parcel, + size_t* size) { + if (parcel == nullptr || size == nullptr) { + _E("Invalid parameter"); + return PARCEL_ERROR_INVALID_PARAMETER; + } + + auto* h = static_cast(parcel); + *size = h->GetDataCapacity(); + return PARCEL_ERROR_NONE; +} + +extern "C" EXPORT int parcel_set_data_capacity(parcel_h parcel, + size_t size) { + if (parcel == nullptr || size == 0) { + _E("Invalid parameter"); + return PARCEL_ERROR_INVALID_PARAMETER; + } + + auto* h = static_cast(parcel); + try { + h->SetDataCapacity(size); + } catch (const std::bad_alloc& e) { + _E("Exception occurs. error(%s)", e.what()); + return PARCEL_ERROR_OUT_OF_MEMORY; + } + + return PARCEL_ERROR_NONE; +} diff --git a/tests/parcel_unittests/test_parcel.cc b/tests/parcel_unittests/test_parcel.cc index e05f553..a2bc1b8 100644 --- a/tests/parcel_unittests/test_parcel.cc +++ b/tests/parcel_unittests/test_parcel.cc @@ -538,3 +538,30 @@ TEST_F(ParcelTest, parcel_set_byte_order_N) { int ret = parcel_set_byte_order(nullptr, false); ASSERT_EQ(ret, PARCEL_ERROR_INVALID_PARAMETER); } + +TEST_F(ParcelTest, parcel_get_data_capacity_P) { + size_t size = 0; + int ret = parcel_get_data_capacity(GetHandle(), &size); + ASSERT_EQ(ret, PARCEL_ERROR_NONE); + ASSERT_NE(size, 0); +} + +TEST_F(ParcelTest, parcel_get_data_capacity_N) { + int ret = parcel_get_data_capacity(nullptr, nullptr); + ASSERT_EQ(ret, PARCEL_ERROR_INVALID_PARAMETER); +} + +TEST_F(ParcelTest, parcel_set_data_capacity_P) { + int ret = parcel_set_data_capacity(GetHandle(), 128); + ASSERT_EQ(ret, PARCEL_ERROR_NONE); + + size_t size = 0; + ret = parcel_get_data_capacity(GetHandle(), &size); + ASSERT_EQ(ret, PARCEL_ERROR_NONE); + ASSERT_EQ(size, 128); +} + +TEST_F(ParcelTest, parcel_set_data_capacity_N) { + int ret = parcel_set_data_capacity(nullptr, 0); + ASSERT_EQ(ret, PARCEL_ERROR_INVALID_PARAMETER); +} diff --git a/tests/parcel_unittests/test_parcel_cpp.cc b/tests/parcel_unittests/test_parcel_cpp.cc index ff52fb1..3c6b73a 100644 --- a/tests/parcel_unittests/test_parcel_cpp.cc +++ b/tests/parcel_unittests/test_parcel_cpp.cc @@ -184,12 +184,6 @@ TEST_F(ParcelCppTest, WriteCString_AND_ReadCString) { ASSERT_EQ(std::string(str), "TestCString"); } -TEST_F(ParcelCppTest, GetRaw) { - GetHandle().WriteInt32(0); - auto& raw = GetHandle().GetRaw(); - ASSERT_EQ(raw.size(), sizeof(int32_t)); -} - TEST_F(ParcelCppTest, ResetReader) { GetHandle().WriteInt32(1); GetHandle().WriteInt32(2); @@ -246,3 +240,50 @@ TEST_F(ParcelCppTest, SetByteOrder) { ASSERT_EQ(ret, Parcel::Error::None); ASSERT_EQ(val, 1); } + +TEST_F(ParcelCppTest, GetData) { + uint8_t* data = GetHandle().GetData(); + ASSERT_NE(data, nullptr); +} + +TEST_F(ParcelCppTest, GetDataSize) { + GetHandle().WriteInt32(1); + ASSERT_EQ(GetHandle().GetDataSize(), sizeof(int32_t)); + GetHandle().WriteInt32(2); + ASSERT_EQ(GetHandle().GetDataSize(), sizeof(int32_t) + sizeof(int32_t)); +} + +TEST_F(ParcelCppTest, GetDataCapacity) { + size_t size = GetHandle().GetDataCapacity(); + ASSERT_NE(size, 0); +} + +TEST_F(ParcelCppTest, SetDataCapacity) { + GetHandle().SetDataCapacity(128); + size_t size = GetHandle().GetDataCapacity(); + ASSERT_EQ(size, 128); +} + +TEST_F(ParcelCppTest, Detach) { + GetHandle().WriteInt32(1); + size_t data_size = 0; + uint8_t* data = GetHandle().Detach(&data_size); + std::unique_ptr data_auto(data, std::free); + ASSERT_NE(data, nullptr); + ASSERT_EQ(data_size, sizeof(int32_t)); +} + +TEST_F(ParcelCppTest, GetReader) { + GetHandle().WriteInt32(1); + GetHandle().WriteString("GetReader"); + int32_t value = 0; + GetHandle().ReadInt32(&value); + uint32_t reader = GetHandle().GetReader(); + ASSERT_EQ(reader, sizeof(int32_t)); +} + +TEST_F(ParcelCppTest, ToRaw) { + GetHandle().WriteInt32(0); + auto raw = GetHandle().ToRaw(); + ASSERT_EQ(raw.size(), sizeof(int32_t)); +} -- 2.7.4