pwrliang commented on code in PR #556:
URL: https://github.com/apache/sedona-db/pull/556#discussion_r2760735434
##########
c/sedona-libgpuspatial/libgpuspatial/include/gpuspatial/loader/parallel_wkb_loader.hpp:
##########
@@ -756,102 +799,99 @@ class ParallelWkbLoader {
private:
Config config_;
- ArrowArrayView array_view_;
+ nanoarrow::UniqueArrayView array_view_;
GeometryType geometry_type_;
detail::DeviceParsedGeometries<POINT_T, INDEX_T> geoms_;
std::shared_ptr<ThreadPool> thread_pool_;
- void updateGeometryType(int64_t offset, int64_t length) {
+ template <typename OFFSET_IT>
+ void updateGeometryType(OFFSET_IT begin, OFFSET_IT end) {
if (geometry_type_ == GeometryType::kGeometryCollection) {
- // it's already the most generic type
return;
}
- std::vector<bool> type_flags(8 /*WKB types*/, false);
- std::vector<std::thread> workers;
+ size_t num_offsets = std::distance(begin, end);
+ if (num_offsets == 0) return;
+
auto parallelism = thread_pool_->num_threads();
- auto thread_work_size = (length + parallelism - 1) / parallelism;
- std::vector<std::future<void>> futures;
+ auto thread_work_size = (num_offsets + parallelism - 1) / parallelism;
+
+ std::vector<std::future<uint32_t>> futures;
+ futures.reserve(parallelism);
+
+ // Detect Endianness once (outside the loop)
+ const bool host_is_little = detail::is_little_endian();
for (int thread_idx = 0; thread_idx < parallelism; thread_idx++) {
- auto run = [&](int tid) {
- auto thread_work_start = tid * thread_work_size;
- auto thread_work_end = std::min(length, thread_work_start +
thread_work_size);
- GeoArrowWKBReader reader;
- GeoArrowError error;
- GEOARROW_THROW_NOT_OK(nullptr, GeoArrowWKBReaderInit(&reader));
+ auto run = [=](int tid) -> uint32_t {
+ size_t thread_work_start = tid * thread_work_size;
+ size_t thread_work_end =
+ std::min(num_offsets, thread_work_start + thread_work_size);
+
+ uint32_t local_seen_mask = 0;
for (uint32_t work_offset = thread_work_start; work_offset <
thread_work_end;
work_offset++) {
- auto arrow_offset = work_offset + offset;
- // handle null value
- if (ArrowArrayViewIsNull(&array_view_, arrow_offset)) {
+ auto arrow_offset = begin[work_offset];
+
+ if (ArrowArrayViewIsNull(array_view_.get(), arrow_offset)) {
Review Comment:
Thanks for your suggestions. I think it is not worth many lines of code to
do this, since I have measured that this function does not take much runtime.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]