Partial clean up of ORC writer #7324
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ug-orc-null-bool
…ug-orc-null-bool
…ug-orc-null-bool
…efactor-orc-writer-host
vuule
added
cuIO
Codecov Report
@@ Coverage Diff @@
## branch-0.19 #7324 +/- ##
===============================================
+ Coverage 81.80% 81.86% +0.05%
===============================================
Files 101 101
Lines 16695 16884 +189
===============================================
+ Hits 13658 13822 +164
- Misses 3037 3062 +25
Continue to review full report at Codecov.
|
…efactor-orc-writer-host
…efactor-orc-writer-host
jrhemstad
reviewed
Feb 23, 2021
|
Added @cwharris in case he wants to review the span related changes. |
jrhemstad
approved these changes
Mar 1, 2021
vuule
commented
Mar 2, 2021
Comment on lines
+378
to
+379
| std::accumulate(stripe_bounds.front().cbegin(), | ||
| stripe_bounds.back().cend(), |
There was a problem hiding this comment.
This look odd, maybe I should rename these APIs.
add missing newline
rgsl888prabhu
approved these changes
Mar 3, 2021
Merged
…efactor-orc-writer-host
kaatish
reviewed
Mar 3, 2021
|
|
||
| operator cudf::detail::host_span<T>() { return {h_data, max_elements}; } | ||
| operator cudf::detail::host_span<T const>() const { return {h_data, max_elements}; } | ||
|
|
There was a problem hiding this comment.
It might also make sense to store the device side data of hostdevice_vector in a rmm::device_uvector instead of a device_buffer. This will probably re-use the device_span constructor
template <typename C, std::enable_if_t<is_device_span_supported_container<C>::value>* = nullptr>
constexpr device_span(C& in) : base(thrust::raw_pointer_cast(in.data()), in.size())
There was a problem hiding this comment.
AFAICT, we would still need the conversion operators to device_span here, but we wouldn't have to pass the size separately. I do like this change in general, we should look into polishing hostdevice_vector at some point.
|
@gpucibot merge |
hyperbolic2346
pushed a commit
to hyperbolic2346/cudf
that referenced
this pull request
Mar 25, 2021
Issue rapidsai#6763 Clean up of the code surrounding the column data encode in the ORC writer: 1. Add a 2D version of `hostdevice_vector` (single allocation); 2. Add 2D versions of `host_span` and `device_span`; 3. Add implicit conversions from `hostdevice_vector` to `host_span` and `device_span`. 4. Use the new types to represent collections that currently use flattened `hostdevice_vectors`; 5. Separated a part of `EncChunk` into a separate class, `encoder_chunk_streams`, as this is the only part used after data encode; 6. Add `orc_streams` to represent per-column streams and compute offsets. 7. Partial `writer_impl.cu` code "modernization". 8. Removed redundant size parameters (since 2dspan and 2dvector hold the size info). 9. use `device_uvector` instead of `device_vector`. Authors: - Vukasin Milovanovic (@vuule) Approvers: - Jake Hemstad (@jrhemstad) - Kumar Aatish (@kaatish) - Ram (Ramakrishna Prabhu) (@rgsl888prabhu) URL: rapidsai#7324
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #6763
Clean up of the code surrounding the column data encode in the ORC writer:
hostdevice_vector(single allocation);host_spananddevice_span;hostdevice_vectortohost_spananddevice_span.hostdevice_vectors;EncChunkinto a separate class,encoder_chunk_streams, as this is the only part used after data encode;orc_streamsto represent per-column streams and compute offsets.writer_impl.cucode "modernization".device_uvectorinstead ofdevice_vector.