github-actions[bot] commented on code in PR #33622:
URL: https://github.com/apache/doris/pull/33622#discussion_r1564769475
##########
be/src/util/block_compression.cpp:
##########
@@ -751,14 +739,30 @@
// for ZSTD compression and decompression, with BOTH fast and high compression
ratio
class ZstdBlockCompression : public BlockCompressionCodec {
private:
- struct CContext {
+ class CContext {
+ ENABLE_FACTORY_CREATOR(CContext);
+
+ public:
CContext() : ctx(nullptr) {}
ZSTD_CCtx* ctx;
faststring buffer;
+ ~CContext() {
+ if (ctx) {
+ ZSTD_freeCCtx(ctx);
+ }
+ }
};
- struct DContext {
+ class DContext {
+ ENABLE_FACTORY_CREATOR(DContext);
+
+ public:
Review Comment:
warning: use '= default' to define a trivial default constructor
[modernize-use-equals-default]
```suggestion
DContext() : ctx(nullptr) = default;
```
##########
be/src/util/block_compression.cpp:
##########
@@ -90,22 +91,26 @@ bool BlockCompressionCodec::exceed_max_compress_len(size_t
uncompressed_size) {
class Lz4BlockCompression : public BlockCompressionCodec {
private:
- struct Context {
+ class Context {
+ ENABLE_FACTORY_CREATOR(Context);
+
+ public:
Review Comment:
warning: use '= default' to define a trivial default constructor
[modernize-use-equals-default]
```suggestion
Context() : ctx(nullptr) = default;
```
##########
be/src/util/block_compression.cpp:
##########
@@ -233,14 +230,30 @@
// Used for LZ4 frame format, decompress speed is two times faster than LZ4.
class Lz4fBlockCompression : public BlockCompressionCodec {
private:
- struct CContext {
+ class CContext {
+ ENABLE_FACTORY_CREATOR(CContext);
+
+ public:
CContext() : ctx(nullptr) {}
LZ4F_compressionContext_t ctx;
faststring buffer;
+ ~CContext() {
+ if (ctx) {
+ LZ4F_freeCompressionContext(ctx);
+ }
+ }
};
- struct DContext {
+ class DContext {
+ ENABLE_FACTORY_CREATOR(DContext);
+
+ public:
Review Comment:
warning: use '= default' to define a trivial default constructor
[modernize-use-equals-default]
```suggestion
DContext() : ctx(nullptr) = default;
```
##########
be/src/util/block_compression.cpp:
##########
@@ -457,6 +456,7 @@
class Lz4HCBlockCompression : public BlockCompressionCodec {
private:
struct Context {
+ ENABLE_FACTORY_CREATOR(Context);
Review Comment:
warning: use '= default' to define a trivial default constructor
[modernize-use-equals-default]
```suggestion
Context() : ctx(nullptr) = default;
```
##########
be/src/util/block_compression.cpp:
##########
@@ -525,41 +519,35 @@
size_t max_compressed_len(size_t len) override { return
LZ4_compressBound(len); }
private:
- Status _acquire_compression_ctx(Context** out) {
+ Status _acquire_compression_ctx(std::shared_ptr<Context>& out) {
std::lock_guard<std::mutex> l(_ctx_mutex);
if (_ctx_pool.empty()) {
- Context* context = new (std::nothrow) Context();
- if (context == nullptr) {
+ std::shared_ptr<Context> localCtx = Context::create_shared();
+ if (localCtx.get() == nullptr) {
return Status::InvalidArgument("new LZ4HC context error");
}
- context->ctx = LZ4_createStreamHC();
- if (context->ctx == nullptr) {
- delete context;
+ localCtx->ctx = LZ4_createStreamHC();
+ if (localCtx->ctx == nullptr) {
return Status::InvalidArgument("LZ4_createStreamHC error");
}
- *out = context;
+ out = localCtx;
return Status::OK();
}
- *out = _ctx_pool.back();
+ out = _ctx_pool.back();
_ctx_pool.pop_back();
return Status::OK();
}
- void _release_compression_ctx(Context* context) {
+ void _release_compression_ctx(std::shared_ptr<Context> context) {
DCHECK(context);
LZ4_resetStreamHC_fast(context->ctx, _compression_level);
std::lock_guard<std::mutex> l(_ctx_mutex);
_ctx_pool.push_back(context);
}
- void _delete_compression_ctx(Context* context) {
- DCHECK(context);
- LZ4_freeStreamHC(context->ctx);
- delete context;
- }
private:
int64_t _compression_level = config::LZ4_HC_compression_level;
Review Comment:
warning: redundant access specifier has the same accessibility as the
previous access specifier [readability-redundant-access-specifiers]
```suggestion
```
<details>
<summary>Additional context</summary>
**be/src/util/block_compression.cpp:521:** previously declared here
```cpp
private:
^
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
