This is an automated email from the ASF dual-hosted git repository.
maxyang pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudberry.git
The following commit(s) were added to refs/heads/main by this push:
new c50346365e1 PAX: Use const reference to avoid constructor/destructor
of shared pointer
c50346365e1 is described below
commit c50346365e1144d64d08f3e38f1cd032f1cc0f55
Author: Hao Wu <[email protected]>
AuthorDate: Thu Jul 10 08:42:35 2025 +0000
PAX: Use const reference to avoid constructor/destructor of shared pointer
Accessing the visibility bitmap of a column is temporary. Frequently
building the shared pointer of the bitmap and destroy it will hurt
performance.
This commit uses std::unique_ptr instead of std::shared_ptr
Note: The returned left reference should not hold long time, its
lifetime must be shorter than the corresponding column.
---
.../pax_storage/src/api/python3/paxfilereader_type.cc | 5 ++---
.../pax_storage/src/cpp/storage/columns/pax_column.cc | 2 +-
.../pax_storage/src/cpp/storage/columns/pax_column.h | 6 +++---
.../pax_storage/src/cpp/storage/columns/pax_columns.cc | 8 ++++----
.../src/cpp/storage/orc/orc_format_reader.cc | 9 ++++-----
contrib/pax_storage/src/cpp/storage/orc/orc_group.cc | 6 +++---
contrib/pax_storage/src/cpp/storage/orc/orc_reader.cc | 2 +-
.../pax_storage/src/cpp/storage/orc/orc_vec_group.cc | 2 +-
contrib/pax_storage/src/cpp/storage/toast/pax_toast.cc | 18 +++++++++---------
contrib/pax_storage/src/cpp/storage/toast/pax_toast.h | 2 +-
.../src/cpp/storage/vec/pax_porc_adpater.cc | 12 ++++++------
.../pax_storage/src/cpp/storage/vec/pax_vec_comm.cc | 4 ++--
12 files changed, 37 insertions(+), 39 deletions(-)
diff --git a/contrib/pax_storage/src/api/python3/paxfilereader_type.cc
b/contrib/pax_storage/src/api/python3/paxfilereader_type.cc
index 67d58a99ac9..3898aee2cab 100644
--- a/contrib/pax_storage/src/api/python3/paxfilereader_type.cc
+++ b/contrib/pax_storage/src/api/python3/paxfilereader_type.cc
@@ -323,7 +323,6 @@ static PyObject
*paxfilereader_readgroup(PaxFileReaderObject *self,
try {
for (; column_index < col_nums; column_index++) {
const auto &column = (*columns)[column_index];
- std::shared_ptr<pax::Bitmap8> bm;
auto null_counts = 0;
PyObject *schema_item = nullptr;
long col_oid;
@@ -351,7 +350,7 @@ static PyObject
*paxfilereader_readgroup(PaxFileReaderObject *self,
continue;
}
- bm = column->GetBitmap();
+ const auto &bm = column->GetBitmap();
py_rows = PyList_New(0);
if (!py_rows) {
@@ -381,7 +380,7 @@ static PyObject
*paxfilereader_readgroup(PaxFileReaderObject *self,
if (column->IsToast(row_index)) {
// safe to no keep the ref, because paxbuffer_to_pyobj will copy
the
// detoast datum
- std::shared_ptr<pax::MemoryObject> ref = nullptr;
+ std::unique_ptr<pax::MemoryObject> ref = nullptr;
auto datum = PointerGetDatum(buff);
auto external_buffer = column->GetExternalToastDataBuffer();
diff --git a/contrib/pax_storage/src/cpp/storage/columns/pax_column.cc
b/contrib/pax_storage/src/cpp/storage/columns/pax_column.cc
index 2d4bd6b7d0f..7d4daf6aaf0 100644
--- a/contrib/pax_storage/src/cpp/storage/columns/pax_column.cc
+++ b/contrib/pax_storage/src/cpp/storage/columns/pax_column.cc
@@ -83,7 +83,7 @@ size_t PaxColumn::GetRangeNonNullRows(size_t start_pos,
size_t len) {
void PaxColumn::CreateNulls(size_t cap) {
Assert(!null_bitmap_);
- null_bitmap_ = std::make_shared<Bitmap8>(cap);
+ null_bitmap_ = std::make_unique<Bitmap8>(cap);
null_bitmap_->SetN(total_rows_);
}
diff --git a/contrib/pax_storage/src/cpp/storage/columns/pax_column.h
b/contrib/pax_storage/src/cpp/storage/columns/pax_column.h
index f17320c0441..b713dbc73fb 100644
--- a/contrib/pax_storage/src/cpp/storage/columns/pax_column.h
+++ b/contrib/pax_storage/src/cpp/storage/columns/pax_column.h
@@ -233,13 +233,13 @@ class PaxColumn {
inline bool AllNull() const { return null_bitmap_ && null_bitmap_->Empty(); }
// Set the null bitmap
- inline void SetBitmap(std::shared_ptr<Bitmap8> null_bitmap) {
+ inline void SetBitmap(std::unique_ptr<Bitmap8> null_bitmap) {
Assert(!null_bitmap_);
null_bitmap_ = std::move(null_bitmap);
}
// Get the null bitmap
- inline std::shared_ptr<Bitmap8> GetBitmap() const { return null_bitmap_; }
+ inline const std::unique_ptr<Bitmap8>& GetBitmap() const { return
null_bitmap_; }
// Set the column kv attributes
void SetAttributes(const std::map<std::string, std::string> &attrs);
@@ -354,7 +354,7 @@ class PaxColumn {
protected:
// null field bit map
- std::shared_ptr<Bitmap8> null_bitmap_;
+ std::unique_ptr<Bitmap8> null_bitmap_;
// Writer: write pointer
// Reader: total rows
diff --git a/contrib/pax_storage/src/cpp/storage/columns/pax_columns.cc
b/contrib/pax_storage/src/cpp/storage/columns/pax_columns.cc
index 4a181947ed8..7fd3c04ae60 100644
--- a/contrib/pax_storage/src/cpp/storage/columns/pax_columns.cc
+++ b/contrib/pax_storage/src/cpp/storage/columns/pax_columns.cc
@@ -377,7 +377,7 @@ size_t PaxColumns::MeasureVecDataBuffer(
// has null will generate a bitmap in current stripe
if (column->HasNull()) {
- auto bm = column->GetBitmap();
+ const auto &bm = column->GetBitmap();
Assert(bm);
size_t bm_length = bm->MinimalStoredBytes(total_rows);
@@ -483,7 +483,7 @@ size_t PaxColumns::MeasureOrcDataBuffer(
auto column = p_column.get();
// has null will generate a bitmap in current stripe
if (column->HasNull()) {
- auto bm = column->GetBitmap();
+ const auto &bm = column->GetBitmap();
Assert(bm);
size_t bm_length = bm->MinimalStoredBytes(column->GetRows());
buffer_len += bm_length;
@@ -592,7 +592,7 @@ void PaxColumns::CombineVecDataBuffer() {
auto column = p_column.get();
if (column->HasNull()) {
- auto bm = column->GetBitmap();
+ const auto &bm = column->GetBitmap();
Assert(bm);
auto nbytes = bm->MinimalStoredBytes(column->GetRows());
Assert(nbytes <= bm->Raw().size);
@@ -680,7 +680,7 @@ void PaxColumns::CombineOrcDataBuffer() {
auto column = p_column.get();
if (column->HasNull()) {
- auto bm = column->GetBitmap();
+ const auto &bm = column->GetBitmap();
Assert(bm);
auto nbytes = bm->MinimalStoredBytes(column->GetRows());
Assert(nbytes <= bm->Raw().size);
diff --git a/contrib/pax_storage/src/cpp/storage/orc/orc_format_reader.cc
b/contrib/pax_storage/src/cpp/storage/orc/orc_format_reader.cc
index 5ca1670e5ef..94ac27bbf4c 100644
--- a/contrib/pax_storage/src/cpp/storage/orc/orc_format_reader.cc
+++ b/contrib/pax_storage/src/cpp/storage/orc/orc_format_reader.cc
@@ -899,7 +899,7 @@ std::unique_ptr<PaxColumns> OrcFormatReader::ReadStripe(
continue;
}
- std::shared_ptr<Bitmap8> non_null_bitmap;
+ std::unique_ptr<Bitmap8> non_null_bitmap;
bool has_null = stripe_info.colstats(index).hasnull();
if (has_null) {
const pax::porc::proto::Stream &non_null_stream =
@@ -909,9 +909,8 @@ std::unique_ptr<PaxColumns> OrcFormatReader::ReadStripe(
reinterpret_cast<uint8 *>(data_buffer->GetAvailableBuffer());
Assert(non_null_stream.kind() == pax::porc::proto::Stream_Kind_PRESENT);
- non_null_bitmap =
- std::make_shared<Bitmap8>(BitmapRaw<uint8>(bm_bytes, bm_nbytes),
- BitmapTpl<uint8>::ReadOnlyRefBitmap);
+ non_null_bitmap = std::make_unique<Bitmap8>(BitmapRaw<uint8>(bm_bytes,
bm_nbytes),
+ BitmapTpl<uint8>::ReadOnlyRefBitmap);
data_buffer->Brush(bm_nbytes);
}
@@ -1023,7 +1022,7 @@ std::unique_ptr<PaxColumns> OrcFormatReader::ReadStripe(
last_column->SetRows(stripe_info.numberofrows());
if (has_null) {
Assert(non_null_bitmap);
- last_column->SetBitmap(non_null_bitmap);
+ last_column->SetBitmap(std::move(non_null_bitmap));
}
if (!column_attrs_[index].empty()) {
diff --git a/contrib/pax_storage/src/cpp/storage/orc/orc_group.cc
b/contrib/pax_storage/src/cpp/storage/orc/orc_group.cc
index 276b640ca1f..d7c8c73d220 100644
--- a/contrib/pax_storage/src/cpp/storage/orc/orc_group.cc
+++ b/contrib/pax_storage/src/cpp/storage/orc/orc_group.cc
@@ -49,7 +49,7 @@ inline static std::pair<Datum, bool> GetColumnDatum(PaxColumn
*column,
Datum rc;
if (column->HasNull()) {
- auto bm = column->GetBitmap();
+ const auto &bm = column->GetBitmap();
Assert(bm);
if (!bm->Test(row_index)) {
*null_counts += 1;
@@ -123,7 +123,7 @@ std::pair<bool, size_t> OrcGroup::ReadTuple(TupleTableSlot
*slot) {
}
if (column->HasNull()) {
- auto bm = column->GetBitmap();
+ const auto &bm = column->GetBitmap();
Assert(bm);
if (!bm->Test(current_row_index_)) {
current_nulls_[index]++;
@@ -307,7 +307,7 @@ std::pair<Datum, bool>
OrcGroup::GetColumnValueNoMissing(size_t column_index,
void OrcGroup::CalcNullShuffle(PaxColumn *column, size_t column_index) {
auto rows = column->GetRows();
uint32 n_counts = 0;
- auto bm = column->GetBitmap();
+ const auto &bm = column->GetBitmap();
Assert(bm);
Assert(column->HasNull() && !nulls_shuffle_[column_index]);
diff --git a/contrib/pax_storage/src/cpp/storage/orc/orc_reader.cc
b/contrib/pax_storage/src/cpp/storage/orc/orc_reader.cc
index 953debe25dd..3fa4192efc6 100644
--- a/contrib/pax_storage/src/cpp/storage/orc/orc_reader.cc
+++ b/contrib/pax_storage/src/cpp/storage/orc/orc_reader.cc
@@ -129,7 +129,7 @@ std::unique_ptr<MicroPartitionReader::Group>
OrcReader::ReadGroup(
for (size_t i = 0; i < pax_columns->GetColumns(); i++) {
auto column = (*pax_columns)[i].get();
if (column && !column->GetBuffer().first) {
- auto bm = column->GetBitmap();
+ const auto &bm = column->GetBitmap();
// Assert(bm);
if (bm) {
for (size_t n = 0; n < column->GetRows(); n++) {
diff --git a/contrib/pax_storage/src/cpp/storage/orc/orc_vec_group.cc
b/contrib/pax_storage/src/cpp/storage/orc/orc_vec_group.cc
index efeecdccb3c..b8690178109 100644
--- a/contrib/pax_storage/src/cpp/storage/orc/orc_vec_group.cc
+++ b/contrib/pax_storage/src/cpp/storage/orc/orc_vec_group.cc
@@ -32,7 +32,7 @@ namespace pax {
inline static std::pair<Datum, bool> GetColumnDatum(PaxColumn *column,
size_t row_index) {
if (column->HasNull()) {
- auto bm = column->GetBitmap();
+ const auto &bm = column->GetBitmap();
Assert(bm);
if (!bm->Test(row_index)) {
return {0, true};
diff --git a/contrib/pax_storage/src/cpp/storage/toast/pax_toast.cc
b/contrib/pax_storage/src/cpp/storage/toast/pax_toast.cc
index 3aa54e48ea5..f51bee8a07f 100644
--- a/contrib/pax_storage/src/cpp/storage/toast/pax_toast.cc
+++ b/contrib/pax_storage/src/cpp/storage/toast/pax_toast.cc
@@ -495,15 +495,15 @@ size_t pax_detoast_raw(Datum d, char *dst_buff, size_t
dst_cap, char *ext_buff,
return decompress_size;
}
-std::pair<Datum, std::shared_ptr<MemoryObject>> pax_detoast(
+std::pair<Datum, std::unique_ptr<MemoryObject>> pax_detoast(
Datum d, char *ext_buff, size_t ext_buff_size) {
- std::shared_ptr<ExternalToastValue> value;
+ std::unique_ptr<ExternalToastValue> value;
if (VARATT_IS_COMPRESSED(d)) {
char *result;
size_t raw_size = VARDATA_COMPRESSED_GET_EXTSIZE(d);
- value = std::make_shared<ExternalToastValue>(raw_size + VARHDRSZ);
+ value = std::make_unique<ExternalToastValue>(raw_size + VARHDRSZ);
result = reinterpret_cast<char *>(value->Addr());
// only external toast exist invalid compress toast
Assert((ToastCompressionId)(TOAST_COMPRESS_METHOD(d)) !=
@@ -515,8 +515,8 @@ std::pair<Datum, std::shared_ptr<MemoryObject>> pax_detoast(
SET_VARSIZE(result, raw_size + VARHDRSZ);
- return std::pair<Datum, std::shared_ptr<MemoryObject>>{
- PointerGetDatum(result), value};
+ return std::pair<Datum, std::unique_ptr<MemoryObject>>{
+ PointerGetDatum(result), std::move(value)};
} else if (VARATT_IS_PAX_EXTERNAL_TOAST(d)) {
char *result;
Assert(ext_buff);
@@ -531,7 +531,7 @@ std::pair<Datum, std::shared_ptr<MemoryObject>> pax_detoast(
"buff size=%lu]",
offset, raw_size, ext_buff_size));
- value = std::make_shared<ExternalToastValue>(origin_size + VARHDRSZ);
+ value = std::make_unique<ExternalToastValue>(origin_size + VARHDRSZ);
result = reinterpret_cast<char *>(value->Addr());
auto pg_attribute_unused() decompress_size =
@@ -540,11 +540,11 @@ std::pair<Datum, std::shared_ptr<MemoryObject>>
pax_detoast(
Assert(decompress_size == origin_size);
SET_VARSIZE(result, origin_size + VARHDRSZ);
- return std::pair<Datum, std::shared_ptr<MemoryObject>>{
- PointerGetDatum(result), value};
+ return std::pair<Datum, std::unique_ptr<MemoryObject>>{
+ PointerGetDatum(result), std::move(value)};
}
- return std::pair<Datum, std::shared_ptr<MemoryObject>>{d, nullptr};
+ return std::pair<Datum, std::unique_ptr<MemoryObject>>{d, nullptr};
}
ExternalToastValue::ExternalToastValue(size_t size)
diff --git a/contrib/pax_storage/src/cpp/storage/toast/pax_toast.h
b/contrib/pax_storage/src/cpp/storage/toast/pax_toast.h
index 74d5271b99f..f7825b45256 100644
--- a/contrib/pax_storage/src/cpp/storage/toast/pax_toast.h
+++ b/contrib/pax_storage/src/cpp/storage/toast/pax_toast.h
@@ -145,7 +145,7 @@ size_t pax_toast_hdr_size(Datum d);
// detoast pax toast
size_t pax_detoast_raw(Datum d, char *dst_buff, size_t dst_size,
char *ext_buff = nullptr, size_t ext_buff_size = 0);
-std::pair<Datum, std::shared_ptr<MemoryObject>> pax_detoast(
+std::pair<Datum, std::unique_ptr<MemoryObject>> pax_detoast(
Datum d, char *ext_buff = nullptr, size_t ext_buff_size = 0);
// free pax toast
diff --git a/contrib/pax_storage/src/cpp/storage/vec/pax_porc_adpater.cc
b/contrib/pax_storage/src/cpp/storage/vec/pax_porc_adpater.cc
index 3e77c04bc1a..9cfcc49b091 100644
--- a/contrib/pax_storage/src/cpp/storage/vec/pax_porc_adpater.cc
+++ b/contrib/pax_storage/src/cpp/storage/vec/pax_porc_adpater.cc
@@ -49,7 +49,7 @@ static void CopyFixedRawBufferWithNull(
std::tie(buffer, buffer_len) =
column->GetRangeBuffer(data_index_begin, data_range_lens);
- auto null_bitmap = column->GetBitmap();
+ const auto &null_bitmap = column->GetBitmap();
size_t non_null_offset = 0;
size_t type_len = column->GetTypeLength();
for (size_t i = range_begin; i < (range_begin + range_lens); i++) {
@@ -121,10 +121,10 @@ static void CopyNonFixedBuffer(PaxColumn *column,
size_t dst_offset = out_data_buffer->Used();
char *buffer = nullptr;
size_t buffer_len = 0;
-
- auto null_bitmap = column->GetBitmap();
size_t non_null_offset = 0;
+ const auto &null_bitmap = column->GetBitmap();
+
for (size_t i = range_begin; i < (range_begin + range_lens); i++) {
if (visibility_map_bitset &&
visibility_map_bitset->Test(i - range_begin + bitset_index_begin)) {
@@ -219,7 +219,7 @@ static void CopyDecimalBuffer(PaxColumn *column,
size_t buffer_len = 0;
int32 type_len;
- auto null_bitmap = column->GetBitmap();
+ const auto &null_bitmap = column->GetBitmap();
type_len = VEC_SHORT_NUMERIC_STORE_BYTES;
for (size_t i = range_begin; i < (range_begin + range_lens); i++) {
@@ -279,11 +279,11 @@ void CopyBitPackedBuffer(PaxColumn *column,
std::tie(buffer, buffer_len) =
column->GetRangeBuffer(data_index_begin, data_range_lens);
- auto null_bitmap = column->GetBitmap();
size_t bit_index = 0;
size_t non_null_offset = 0;
size_t type_len = column->GetTypeLength();
size_t tuple_offset = group_base_offset + range_begin;
+ const auto &null_bitmap = column->GetBitmap();
for (size_t i = 0; i < range_lens; i++) {
bool is_visible = !visibility_map_bitset ||
@@ -431,7 +431,7 @@ std::pair<size_t, size_t>
VecAdapter::AppendPorcFormat(PaxColumns *columns,
vec_cache_buffer_[index].is_dict = true;
if (column->HasNull()) {
- auto null_bitmap = column->GetBitmap();
+ const auto &null_bitmap = column->GetBitmap();
size_t non_null_offset = 0;
for (size_t i = 0; i < range_lens; i++) {
diff --git a/contrib/pax_storage/src/cpp/storage/vec/pax_vec_comm.cc
b/contrib/pax_storage/src/cpp/storage/vec/pax_vec_comm.cc
index 160d2928b9b..11f31caad95 100644
--- a/contrib/pax_storage/src/cpp/storage/vec/pax_vec_comm.cc
+++ b/contrib/pax_storage/src/cpp/storage/vec/pax_vec_comm.cc
@@ -88,12 +88,12 @@ void CopyBitmapBuffer(PaxColumn *column,
Assert(!null_bits_buffer->GetBuffer());
null_bits_buffer->Set(BlockBuffer::Alloc<char>(null_align_bytes),
null_align_bytes);
- auto bitmap = column->GetBitmap();
+ const auto &bitmap = column->GetBitmap();
Assert(bitmap);
CopyBitmap(bitmap.get(), range_begin, range_lens, null_bits_buffer);
*out_visable_null_counts = range_lens - data_range_lens;
} else {
- auto bitmap = column->GetBitmap();
+ const auto &bitmap = column->GetBitmap();
Assert(bitmap);
Bitmap8 null_bitmap(out_range_lens);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
