You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/06/16 10:06:03 UTC
[GitHub] [incubator-doris] Gabriel39 opened a new pull request, #10195: [Improvement] Use IOBufAsZeroCopyInputStream for bloom filter
Gabriel39 opened a new pull request, #10195:
URL: https://github.com/apache/incubator-doris/pull/10195
# Proposed changes
Issue Number: close #xxx
## Problem Summary:
Describe the overview of changes.
## Checklist(Required)
1. Does it affect the original behavior: (Yes/No/I Don't know)
2. Has unit tests been added: (Yes/No/No Need)
3. Has document been added or modified: (Yes/No/No Need)
4. Does it need to update dependencies: (Yes/No)
5. Are there any changes that cannot be rolled back: (Yes/No)
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] adonis0147 commented on a diff in pull request #10195: [Improvement] Use IOBufAsZeroCopyInputStream for bloom filter
Posted by GitBox <gi...@apache.org>.
adonis0147 commented on code in PR #10195:
URL: https://github.com/apache/incubator-doris/pull/10195#discussion_r899874584
##########
be/src/exprs/block_bloom_filter.hpp:
##########
@@ -40,11 +44,14 @@ class BlockBloomFilter {
explicit BlockBloomFilter();
~BlockBloomFilter();
+ BlockBloomFilter(const BlockBloomFilter&) = delete;
+ void operator=(const BlockBloomFilter&) = delete;
Review Comment:
```suggestion
BlockBloomFilter& operator=(const BlockBloomFilter&) = delete;
```
According to [copy_assignment](https://en.cppreference.com/w/cpp/language/copy_assignment).
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] adonis0147 commented on a diff in pull request #10195: [Improvement] Use IOBufAsZeroCopyInputStream for bloom filter
Posted by GitBox <gi...@apache.org>.
adonis0147 commented on code in PR #10195:
URL: https://github.com/apache/incubator-doris/pull/10195#discussion_r899859683
##########
be/src/exprs/block_bloom_filter_impl.cc:
##########
@@ -83,17 +85,26 @@ Status BlockBloomFilter::init(const int log_space_bytes, uint32_t hash_seed) {
return Status::OK();
}
-Status BlockBloomFilter::init_from_directory(int log_space_bytes, const Slice& directory,
- bool always_false, uint32_t hash_seed) {
+Status BlockBloomFilter::init_from_directory(int log_space_bytes,
+ butil::IOBufAsZeroCopyInputStream* data,
+ const size_t& data_size, bool always_false,
+ uint32_t hash_seed) {
Review Comment:
Two points here:
1. The number of arguments is a little more.
2. The following snippet (100-107) looks a little bit complex.
If you introduce another `struct` to wrap them, you can make the code more cleaner.
This comment aimed to enhance the readability other than functionality.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] cambyzju commented on a diff in pull request #10195: [Improvement] Use IOBufAsZeroCopyInputStream for bloom filter
Posted by GitBox <gi...@apache.org>.
cambyzju commented on code in PR #10195:
URL: https://github.com/apache/incubator-doris/pull/10195#discussion_r899802595
##########
be/src/exprs/block_bloom_filter.hpp:
##########
@@ -202,6 +205,13 @@ class BlockBloomFilter {
return (static_cast<uint64_t>(hash) * m + a) >> 32U;
}
+#ifdef DISALLOW_COPY_AND_ASSIGN
+#undef DISALLOW_COPY_AND_ASSIGN
+#endif
+#define DISALLOW_COPY_AND_ASSIGN(TypeName) \
+ TypeName(const TypeName&) = delete; \
+ void operator=(const TypeName&) = delete
+
DISALLOW_COPY_AND_ASSIGN(BlockBloomFilter);
Review Comment:
```suggestion
BlockBloomFilter(const BlockBloomFilter&) = delete;
void operator=(const BlockBloomFilter&) = delete;
```
just expand it, it looks like more simpler.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] adonis0147 commented on a diff in pull request #10195: [Improvement] Use IOBufAsZeroCopyInputStream for bloom filter
Posted by GitBox <gi...@apache.org>.
adonis0147 commented on code in PR #10195:
URL: https://github.com/apache/incubator-doris/pull/10195#discussion_r899859683
##########
be/src/exprs/block_bloom_filter_impl.cc:
##########
@@ -83,17 +85,26 @@ Status BlockBloomFilter::init(const int log_space_bytes, uint32_t hash_seed) {
return Status::OK();
}
-Status BlockBloomFilter::init_from_directory(int log_space_bytes, const Slice& directory,
- bool always_false, uint32_t hash_seed) {
+Status BlockBloomFilter::init_from_directory(int log_space_bytes,
+ butil::IOBufAsZeroCopyInputStream* data,
+ const size_t& data_size, bool always_false,
+ uint32_t hash_seed) {
Review Comment:
Two points here:
1. The number of arguments is a little more.
2. The following snippet (100-107) looks a little bit complex.
If you introduce another `struct` to wrap them, you can make the code cleaner.
This comment aimed to enhance the readability other than functionality.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] adonis0147 commented on a diff in pull request #10195: [Improvement] Use IOBufAsZeroCopyInputStream for bloom filter
Posted by GitBox <gi...@apache.org>.
adonis0147 commented on code in PR #10195:
URL: https://github.com/apache/incubator-doris/pull/10195#discussion_r899846698
##########
be/src/exprs/block_bloom_filter_impl.cc:
##########
@@ -83,17 +85,26 @@ Status BlockBloomFilter::init(const int log_space_bytes, uint32_t hash_seed) {
return Status::OK();
}
-Status BlockBloomFilter::init_from_directory(int log_space_bytes, const Slice& directory,
- bool always_false, uint32_t hash_seed) {
+Status BlockBloomFilter::init_from_directory(int log_space_bytes,
+ butil::IOBufAsZeroCopyInputStream* data,
+ const size_t& data_size, bool always_false,
+ uint32_t hash_seed) {
Review Comment:
I meant you could create some `struct` like the following shows.
```c++
struct IOBufSlice {
butil::IOBufAsZeroCopyInputStream* data;
const size_t data_size;
};
```
and then pass `IOBufSlice` here.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] Gabriel39 commented on a diff in pull request #10195: [Improvement] Use IOBufAsZeroCopyInputStream for bloom filter
Posted by GitBox <gi...@apache.org>.
Gabriel39 commented on code in PR #10195:
URL: https://github.com/apache/incubator-doris/pull/10195#discussion_r899832593
##########
be/src/exprs/block_bloom_filter_impl.cc:
##########
@@ -83,17 +85,26 @@ Status BlockBloomFilter::init(const int log_space_bytes, uint32_t hash_seed) {
return Status::OK();
}
-Status BlockBloomFilter::init_from_directory(int log_space_bytes, const Slice& directory,
- bool always_false, uint32_t hash_seed) {
+Status BlockBloomFilter::init_from_directory(int log_space_bytes,
+ butil::IOBufAsZeroCopyInputStream* data,
+ const size_t& data_size, bool always_false,
+ uint32_t hash_seed) {
RETURN_IF_ERROR(init_internal(log_space_bytes, hash_seed));
DCHECK(_directory);
- if (directory_size() != directory.size) {
+ if (directory_size() != data_size) {
return Status::InvalidArgument(fmt::format(
"Mismatch in BlockBloomFilter source directory size {} and expected size {}",
- directory.size, directory_size()));
+ data_size, directory_size()));
+ }
+ int size = 0;
+ char* tmp;
+ const void** ptr = (const void**)&tmp;
+ char* data_ptr = reinterpret_cast<char*>(_directory);
+ while (data->Next(ptr, &size)) {
+ memcpy(data_ptr, *ptr, size);
+ data_ptr += size;
Review Comment:
Data blocks in IOBufAsZeroCopyInputStream is non-continuous. This is why we use another memcpy before. This PR delete this redundant memcpy and so we can't use Slice here.
##########
be/src/runtime/runtime_filter_mgr.h:
##########
@@ -68,7 +72,8 @@ class RuntimeFilterMgr {
const TQueryOptions& options, int node_id = -1);
// update filter by remote
- Status update_filter(const PPublishFilterRequest* request, const char* data);
+ Status update_filter(const PPublishFilterRequest* request,
+ butil::IOBufAsZeroCopyInputStream* data);
Review Comment:
`IOBufAsZeroCopyInputStream.Next` is a non-const method so we can't make it const here
##########
be/src/exprs/block_bloom_filter.hpp:
##########
@@ -177,7 +180,7 @@ class BlockBloomFilter {
#endif
// Size of the internal directory structure in bytes.
- int64_t directory_size() const { return 1ULL << log_space_bytes(); }
+ size_t directory_size() const { return 1ULL << log_space_bytes(); }
Review Comment:
It's actually s size_t and we use it to compare with other variables with size_t type everywhere. So I just want to keep consistency with other variables here.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] adonis0147 commented on a diff in pull request #10195: [Improvement] Use IOBufAsZeroCopyInputStream for bloom filter
Posted by GitBox <gi...@apache.org>.
adonis0147 commented on code in PR #10195:
URL: https://github.com/apache/incubator-doris/pull/10195#discussion_r899795015
##########
be/src/exprs/block_bloom_filter.hpp:
##########
@@ -44,8 +47,8 @@ class BlockBloomFilter {
Status init(int log_space_bytes, uint32_t hash_seed);
// Initialize the BlockBloomFilter from a populated "directory" structure.
// Useful for initializing the BlockBloomFilter by de-serializing a custom protobuf message.
- Status init_from_directory(int log_space_bytes, const Slice& directory, bool always_false,
- uint32_t hash_seed);
+ Status init_from_directory(int log_space_bytes, butil::IOBufAsZeroCopyInputStream* data,
+ const size_t& data_size, bool always_false, uint32_t hash_seed);
Review Comment:
Using `const size_t` is enough.
```suggestion
const size_t data_size, bool always_false, uint32_t hash_seed);
```
##########
be/src/exprs/block_bloom_filter.hpp:
##########
@@ -177,7 +180,7 @@ class BlockBloomFilter {
#endif
// Size of the internal directory structure in bytes.
- int64_t directory_size() const { return 1ULL << log_space_bytes(); }
+ size_t directory_size() const { return 1ULL << log_space_bytes(); }
Review Comment:
Why use platform related size here? Using `uint64_t` is better.
```suggestion
uint64_t directory_size() const { return 1ULL << log_space_bytes(); }
Write Preview
```
##########
be/src/exprs/block_bloom_filter_impl.cc:
##########
@@ -83,17 +85,26 @@ Status BlockBloomFilter::init(const int log_space_bytes, uint32_t hash_seed) {
return Status::OK();
}
-Status BlockBloomFilter::init_from_directory(int log_space_bytes, const Slice& directory,
- bool always_false, uint32_t hash_seed) {
+Status BlockBloomFilter::init_from_directory(int log_space_bytes,
+ butil::IOBufAsZeroCopyInputStream* data,
+ const size_t& data_size, bool always_false,
+ uint32_t hash_seed) {
RETURN_IF_ERROR(init_internal(log_space_bytes, hash_seed));
DCHECK(_directory);
- if (directory_size() != directory.size) {
+ if (directory_size() != data_size) {
return Status::InvalidArgument(fmt::format(
"Mismatch in BlockBloomFilter source directory size {} and expected size {}",
- directory.size, directory_size()));
+ data_size, directory_size()));
+ }
+ int size = 0;
+ char* tmp;
+ const void** ptr = (const void**)&tmp;
+ char* data_ptr = reinterpret_cast<char*>(_directory);
+ while (data->Next(ptr, &size)) {
+ memcpy(data_ptr, *ptr, size);
+ data_ptr += size;
Review Comment:
If you create other `Slice`, you can define a member function to hide this logic which looks a little bit complex.
##########
be/src/exprs/block_bloom_filter_impl.cc:
##########
@@ -83,17 +85,26 @@ Status BlockBloomFilter::init(const int log_space_bytes, uint32_t hash_seed) {
return Status::OK();
}
-Status BlockBloomFilter::init_from_directory(int log_space_bytes, const Slice& directory,
- bool always_false, uint32_t hash_seed) {
+Status BlockBloomFilter::init_from_directory(int log_space_bytes,
+ butil::IOBufAsZeroCopyInputStream* data,
+ const size_t& data_size, bool always_false,
+ uint32_t hash_seed) {
Review Comment:
It may not be a good idea to change the interface like that which increases the number of arguments. `data` and `data_size` are related and users will use them at the same time here. Try to create some `struct` like `Slice`.
##########
be/src/exprs/block_bloom_filter.hpp:
##########
@@ -202,6 +205,13 @@ class BlockBloomFilter {
return (static_cast<uint64_t>(hash) * m + a) >> 32U;
}
+#ifdef DISALLOW_COPY_AND_ASSIGN
+#undef DISALLOW_COPY_AND_ASSIGN
+#endif
+#define DISALLOW_COPY_AND_ASSIGN(TypeName) \
+ TypeName(const TypeName&) = delete; \
+ void operator=(const TypeName&) = delete
+
Review Comment:
To much similar macros in the codebase, try to unify them rather than create a new one.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] adonis0147 commented on a diff in pull request #10195: [Improvement] Use IOBufAsZeroCopyInputStream for bloom filter
Posted by GitBox <gi...@apache.org>.
adonis0147 commented on code in PR #10195:
URL: https://github.com/apache/incubator-doris/pull/10195#discussion_r899846698
##########
be/src/exprs/block_bloom_filter_impl.cc:
##########
@@ -83,17 +85,26 @@ Status BlockBloomFilter::init(const int log_space_bytes, uint32_t hash_seed) {
return Status::OK();
}
-Status BlockBloomFilter::init_from_directory(int log_space_bytes, const Slice& directory,
- bool always_false, uint32_t hash_seed) {
+Status BlockBloomFilter::init_from_directory(int log_space_bytes,
+ butil::IOBufAsZeroCopyInputStream* data,
+ const size_t& data_size, bool always_false,
+ uint32_t hash_seed) {
Review Comment:
I meant you could create some `struct` like the following shows.
```c++
struct IOBufSlice {
butil::IOBufAsZeroCopyInputStream* data;
const size_t data_size;
};
```
and then pass `IOBufSlice` here.
After that, you could define custom `memcpy` function to hide the following logic.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] adonis0147 commented on a diff in pull request #10195: [Improvement] Use IOBufAsZeroCopyInputStream for bloom filter
Posted by GitBox <gi...@apache.org>.
adonis0147 commented on code in PR #10195:
URL: https://github.com/apache/incubator-doris/pull/10195#discussion_r899859683
##########
be/src/exprs/block_bloom_filter_impl.cc:
##########
@@ -83,17 +85,26 @@ Status BlockBloomFilter::init(const int log_space_bytes, uint32_t hash_seed) {
return Status::OK();
}
-Status BlockBloomFilter::init_from_directory(int log_space_bytes, const Slice& directory,
- bool always_false, uint32_t hash_seed) {
+Status BlockBloomFilter::init_from_directory(int log_space_bytes,
+ butil::IOBufAsZeroCopyInputStream* data,
+ const size_t& data_size, bool always_false,
+ uint32_t hash_seed) {
Review Comment:
Two points here:
1. The number of arguments a little more.
2. The following snippet (100-107) looks a little bit complex.
If you introduce another `struct` to wrap them, you can make the code more cleaner.
This comment aimed to enhance the readability other than functionality.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] Gabriel39 closed pull request #10195: [Improvement] Use IOBufAsZeroCopyInputStream for bloom filter
Posted by GitBox <gi...@apache.org>.
Gabriel39 closed pull request #10195: [Improvement] Use IOBufAsZeroCopyInputStream for bloom filter
URL: https://github.com/apache/doris/pull/10195
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] cambyzju commented on a diff in pull request #10195: [Improvement] Use IOBufAsZeroCopyInputStream for bloom filter
Posted by GitBox <gi...@apache.org>.
cambyzju commented on code in PR #10195:
URL: https://github.com/apache/incubator-doris/pull/10195#discussion_r899806827
##########
be/src/runtime/runtime_filter_mgr.h:
##########
@@ -68,7 +72,8 @@ class RuntimeFilterMgr {
const TQueryOptions& options, int node_id = -1);
// update filter by remote
- Status update_filter(const PPublishFilterRequest* request, const char* data);
+ Status update_filter(const PPublishFilterRequest* request,
+ butil::IOBufAsZeroCopyInputStream* data);
Review Comment:
```suggestion
const butil::IOBufAsZeroCopyInputStream* data);
```
it is better add const, because we can not modify attachment data.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] adonis0147 commented on a diff in pull request #10195: [Improvement] Use IOBufAsZeroCopyInputStream for bloom filter
Posted by GitBox <gi...@apache.org>.
adonis0147 commented on code in PR #10195:
URL: https://github.com/apache/incubator-doris/pull/10195#discussion_r899836878
##########
be/src/exprs/block_bloom_filter_impl.cc:
##########
@@ -83,17 +85,26 @@ Status BlockBloomFilter::init(const int log_space_bytes, uint32_t hash_seed) {
return Status::OK();
}
-Status BlockBloomFilter::init_from_directory(int log_space_bytes, const Slice& directory,
- bool always_false, uint32_t hash_seed) {
+Status BlockBloomFilter::init_from_directory(int log_space_bytes,
+ butil::IOBufAsZeroCopyInputStream* data,
+ const size_t& data_size, bool always_false,
+ uint32_t hash_seed) {
RETURN_IF_ERROR(init_internal(log_space_bytes, hash_seed));
DCHECK(_directory);
- if (directory_size() != directory.size) {
+ if (directory_size() != data_size) {
return Status::InvalidArgument(fmt::format(
"Mismatch in BlockBloomFilter source directory size {} and expected size {}",
- directory.size, directory_size()));
+ data_size, directory_size()));
+ }
+ int size = 0;
+ char* tmp;
+ const void** ptr = (const void**)&tmp;
+ char* data_ptr = reinterpret_cast<char*>(_directory);
+ while (data->Next(ptr, &size)) {
+ memcpy(data_ptr, *ptr, size);
+ data_ptr += size;
Review Comment:
I didn't mean to use `Slice`. I think you could create another `struct`.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] Gabriel39 commented on a diff in pull request #10195: [Improvement] Use IOBufAsZeroCopyInputStream for bloom filter
Posted by GitBox <gi...@apache.org>.
Gabriel39 commented on code in PR #10195:
URL: https://github.com/apache/incubator-doris/pull/10195#discussion_r899837273
##########
be/src/exprs/block_bloom_filter_impl.cc:
##########
@@ -83,17 +85,26 @@ Status BlockBloomFilter::init(const int log_space_bytes, uint32_t hash_seed) {
return Status::OK();
}
-Status BlockBloomFilter::init_from_directory(int log_space_bytes, const Slice& directory,
- bool always_false, uint32_t hash_seed) {
+Status BlockBloomFilter::init_from_directory(int log_space_bytes,
+ butil::IOBufAsZeroCopyInputStream* data,
+ const size_t& data_size, bool always_false,
+ uint32_t hash_seed) {
Review Comment:
we only provide users SQL interface. For developers, we can't continue to use Slice here because data blocks in IOBuf are not continuous. Besides, I think changing Slice to IOBufAsZeroCopyInputStream is not different from adding an argument here.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] Gabriel39 commented on a diff in pull request #10195: [Improvement] Use IOBufAsZeroCopyInputStream for bloom filter
Posted by GitBox <gi...@apache.org>.
Gabriel39 commented on code in PR #10195:
URL: https://github.com/apache/incubator-doris/pull/10195#discussion_r899853021
##########
be/src/exprs/block_bloom_filter_impl.cc:
##########
@@ -83,17 +85,26 @@ Status BlockBloomFilter::init(const int log_space_bytes, uint32_t hash_seed) {
return Status::OK();
}
-Status BlockBloomFilter::init_from_directory(int log_space_bytes, const Slice& directory,
- bool always_false, uint32_t hash_seed) {
+Status BlockBloomFilter::init_from_directory(int log_space_bytes,
+ butil::IOBufAsZeroCopyInputStream* data,
+ const size_t& data_size, bool always_false,
+ uint32_t hash_seed) {
Review Comment:
I'm not sure if it's necessary. `data_size ` here only means bloom filter's size so it may not be used anywhere. So do we really need to make this abstract? I have some doubts for this.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org