This is an automated email from the ASF dual-hosted git repository.
gfphoenix78 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 43e26c79fb1 Fix memory leak for bitmap in PAX
43e26c79fb1 is described below
commit 43e26c79fb10296fa313e92f23046e70b42efae2
Author: Hao Wu <[email protected]>
AuthorDate: Sun Aug 17 16:26:03 2025 +0800
Fix memory leak for bitmap in PAX
Bitmap should free memory if the memory is allocated by itself.
---
contrib/pax_storage/src/cpp/access/pax_visimap.cc | 3 +-
contrib/pax_storage/src/cpp/comm/bitmap.h | 45 +++++++++-------------
.../src/cpp/storage/orc/orc_format_reader.cc | 3 +-
contrib/pax_storage/src/cpp/storage/pax.cc | 6 +--
.../src/cpp/storage/vec/pax_porc_adpater.cc | 3 +-
.../src/cpp/storage/vec_parallel_pax.cc | 2 +-
6 files changed, 25 insertions(+), 37 deletions(-)
diff --git a/contrib/pax_storage/src/cpp/access/pax_visimap.cc
b/contrib/pax_storage/src/cpp/access/pax_visimap.cc
index b96752d31ba..cdc7dfe763e 100644
--- a/contrib/pax_storage/src/cpp/access/pax_visimap.cc
+++ b/contrib/pax_storage/src/cpp/access/pax_visimap.cc
@@ -143,8 +143,7 @@ bool TestVisimap(Relation rel, const char *visimap_name,
int offset) {
fs = Singleton<LocalFileSystem>::GetInstance();
auto visimap = LoadVisimap(fs, options, file_path);
- auto bm = Bitmap8(BitmapRaw<uint8>(visimap->data(), visimap->size()),
- Bitmap8::ReadOnlyOwnBitmap);
+ auto bm = Bitmap8(BitmapRaw<uint8>(visimap->data(), visimap->size()));
auto is_set = bm.Test(offset);
return !is_set;
}
diff --git a/contrib/pax_storage/src/cpp/comm/bitmap.h
b/contrib/pax_storage/src/cpp/comm/bitmap.h
index 1fc831f1787..793f85d668c 100644
--- a/contrib/pax_storage/src/cpp/comm/bitmap.h
+++ b/contrib/pax_storage/src/cpp/comm/bitmap.h
@@ -147,19 +147,9 @@ struct BitmapRaw final {
raw.bitmap = nullptr;
raw.size = 0;
}
- BitmapRaw &operator=(BitmapRaw) = delete;
BitmapRaw &operator=(BitmapRaw &) = delete;
BitmapRaw &operator=(const BitmapRaw &) = delete;
- BitmapRaw &operator=(BitmapRaw &&raw) {
- if (this != &raw) {
- PAX_DELETE_ARRAY(bitmap);
- bitmap = raw.bitmap;
- size = raw.size;
- raw.bitmap = nullptr;
- raw.size = 0;
- }
- return *this;
- }
+ BitmapRaw &operator=(BitmapRaw &&raw) = delete;
~BitmapRaw() = default;
@@ -171,40 +161,46 @@ template <typename T>
class BitmapTpl final {
public:
using BitmapMemoryPolicy = void (*)(BitmapRaw<T> &, uint32);
- explicit BitmapTpl(uint32 initial_size = 16,
- BitmapMemoryPolicy policy = DefaultBitmapMemoryPolicy) {
+ explicit BitmapTpl(uint32 initial_size = 16) {
static_assert(sizeof(T) == 1 || sizeof(T) == 2 || sizeof(T) == 4 ||
sizeof(T) == 8);
static_assert(BM_WORD_BITS == (1 << BM_WORD_SHIFTS));
- policy_ = policy;
- policy(raw_, Max(initial_size, 16));
+ policy_ = DefaultBitmapMemoryPolicy;
+ policy_(raw_, Max(initial_size, 16));
}
- explicit BitmapTpl(const BitmapRaw<T> &raw, BitmapMemoryPolicy policy) {
+ explicit BitmapTpl(const BitmapRaw<T> &raw) {
static_assert(sizeof(T) == 1 || sizeof(T) == 2 || sizeof(T) == 4 ||
sizeof(T) == 8);
static_assert(BM_WORD_BITS == (1 << BM_WORD_SHIFTS));
- Assert(policy == ReadOnlyRefBitmap || policy == ReadOnlyOwnBitmap);
- policy_ = policy;
+ policy_ = ReadOnlyRefBitmap;
raw_.bitmap = raw.bitmap;
raw_.size = raw.size;
}
BitmapTpl(const BitmapTpl &tpl) = delete;
BitmapTpl(BitmapTpl &&tpl)
- : raw_(std::move(tpl.raw_)), policy_(tpl.policy_) {}
+ : raw_(std::move(tpl.raw_)), policy_(tpl.policy_) {
+ tpl.raw_.bitmap = nullptr;
+ tpl.policy_ = ReadOnlyRefBitmap;
+ }
BitmapTpl(BitmapRaw<T> &&raw)
- : raw_(std::move(raw)), policy_(DefaultBitmapMemoryPolicy) {}
+ : raw_(std::move(raw)), policy_(ReadOnlyRefBitmap) {}
BitmapTpl &operator=(const BitmapTpl &tpl) = delete;
BitmapTpl &operator=(BitmapTpl &&tpl) = delete;
~BitmapTpl() {
// Reference doesn't free the memory
- if (policy_ == ReadOnlyRefBitmap) raw_.bitmap = nullptr;
+ if (policy_ == DefaultBitmapMemoryPolicy)
+ PAX_DELETE_ARRAY(raw_.bitmap);
+ raw_.bitmap = nullptr;
}
std::unique_ptr<BitmapTpl> Clone() const {
+ auto bm = std::make_unique<BitmapTpl>(raw_);
auto p = PAX_NEW_ARRAY<T>(raw_.size);
memcpy(p, raw_.bitmap, sizeof(T) * raw_.size);
- BitmapRaw<T> bm_raw(p, raw_.size);
- return std::make_unique<BitmapTpl>(std::move(bm_raw));
+ bm->raw_.bitmap = p;
+ bm->raw_.size = raw_.size;
+ bm->policy_ = DefaultBitmapMemoryPolicy;
+ return bm;
}
inline size_t WordBits() const { return BM_WORD_BITS; }
@@ -272,9 +268,6 @@ class BitmapTpl final {
// raise
CBDB_RAISE(cbdb::CException::kExTypeInvalidMemoryOperation);
}
- static void ReadOnlyOwnBitmap(BitmapRaw<T> & /*raw*/, uint32 /*index*/) {
- CBDB_RAISE(cbdb::CException::kExTypeInvalidMemoryOperation);
- }
static inline size_t RequireWords(size_t nbits) {
return nbits ? ((nbits - 1) >> BM_WORD_SHIFTS) + 1 : 0;
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 94ac27bbf4c..2fa58790d97 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
@@ -909,8 +909,7 @@ 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_unique<Bitmap8>(BitmapRaw<uint8>(bm_bytes,
bm_nbytes),
- BitmapTpl<uint8>::ReadOnlyRefBitmap);
+ non_null_bitmap = std::make_unique<Bitmap8>(BitmapRaw<uint8>(bm_bytes,
bm_nbytes));
data_buffer->Brush(bm_nbytes);
}
diff --git a/contrib/pax_storage/src/cpp/storage/pax.cc
b/contrib/pax_storage/src/cpp/storage/pax.cc
index d1b2000009c..ab10387c76c 100644
--- a/contrib/pax_storage/src/cpp/storage/pax.cc
+++ b/contrib/pax_storage/src/cpp/storage/pax.cc
@@ -615,8 +615,7 @@ void TableDeleter::DeleteWithVisibilityMap(
auto buffer = LoadVisimap(file_system_, file_system_options_,
visibility_map_filename);
auto visibility_file_bitmap =
- Bitmap8(BitmapRaw<uint8>(buffer->data(), buffer->size()),
- Bitmap8::ReadOnlyOwnBitmap);
+ Bitmap8(BitmapRaw<uint8>(buffer->data(), buffer->size()));
visi_bitmap =
Bitmap8::Union(&visibility_file_bitmap, delete_visi_bitmap.get());
@@ -664,8 +663,7 @@ void TableDeleter::DeleteWithVisibilityMap(
// Update the stats in pax aux table
// Notice that: PAX won't update the stats in group
UpdateStatsInAuxTable(catalog_update, micro_partition_metadata,
- std::make_shared<Bitmap8>(visi_bitmap->Raw(),
-
Bitmap8::ReadOnlyOwnBitmap),
+ std::make_shared<Bitmap8>(visi_bitmap->Raw()),
min_max_col_idxs,
cbdb::GetBloomFilterColumnIndexes(rel_),
stats_updater_projection);
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 9cfcc49b091..65eeb4e88ae 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
@@ -576,8 +576,7 @@ std::pair<size_t, size_t>
VecAdapter::AppendPorcFormat(PaxColumns *columns,
vec_buffer->Set(boolean_buffer, align_size);
Bitmap8 vec_bool_bitmap(
- BitmapRaw<uint8>((uint8 *)(boolean_buffer), align_size),
- BitmapTpl<uint8>::ReadOnlyRefBitmap);
+ BitmapRaw<uint8>((uint8 *)(boolean_buffer), align_size));
CopyBitPackedBuffer(column, micro_partition_visibility_bitmap_,
group_base_offset_, range_begin, range_lens,
diff --git a/contrib/pax_storage/src/cpp/storage/vec_parallel_pax.cc
b/contrib/pax_storage/src/cpp/storage/vec_parallel_pax.cc
index a93d5d17646..632be28f827 100644
--- a/contrib/pax_storage/src/cpp/storage/vec_parallel_pax.cc
+++ b/contrib/pax_storage/src/cpp/storage/vec_parallel_pax.cc
@@ -60,7 +60,7 @@ class MicroPartitionInfo : public MicroPartitionInfoProvider {
if (!visimap_name.empty()) {
visimap = pax::LoadVisimap(file_system, nullptr, visimap_name);
BitmapRaw<uint8_t> raw(visimap->data(), visimap->size());
- bitmap = std::make_unique<Bitmap8>(raw,
BitmapTpl<uint8>::ReadOnlyRefBitmap);
+ bitmap = std::make_unique<Bitmap8>(raw);
}
return {std::move(visimap), std::move(bitmap)};
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
