Add PointIndexCodec, PointDataCodec and other Point changes - #2197
Conversation
595e1de to
cc896f6
Compare
5897076 to
5122547
Compare
5122547 to
0811ad9
Compare
a6ab398 to
cc713c6
Compare
| points::AttributeArray* array = attributeIndex < leaf->attributeSet().size() ? | ||
| &leaf->attributeArray(attributeIndex) : nullptr; | ||
| if (array) { | ||
| auto* pagedStream = getOrCreatePagedStream(pagedStreams, attributeIndex); |
There was a problem hiding this comment.
You call this a lot in all these methods but it looks cache-able outside of the leaf loops?
There was a problem hiding this comment.
Yes, it is. I'm not going to try and change this, because this existing pattern matches how things work in the old multi-pass tree I/O. I will focus more heavily on performance improvements for the new codecs.
There was a problem hiding this comment.
I don't quite follow if you mean that this code won't be used much going forward or that you'll fix it in a subsequent MR? Happy to defer to whatever you think is best, but I think think change would be good to action at some point
There was a problem hiding this comment.
I'm not sure if you're primarily suggesting this to improve the readability or maintenance of the code or because you see opportunities to improve performance.
When we introduce the new codecs, these older codecs will be used less and less as I expect that all new read and writes will go through the newer, more efficient codecs. I don't think we'll be able to completely eliminate these codecs for a long time (if ever), but I would prefer to prioritize spending time on improving the new codecs. And I will happily accept all performance-related feedback when I share the new codecs. :)
| if (requiredPasses > maxRequiredPasses) { | ||
| maxRequiredPasses = requiredPasses; | ||
| } | ||
| } |
There was a problem hiding this comment.
Again, going to keep this as-is to match the existing tree I/O implementation. Optimization effort can focus on new codecs.
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
Signed-off-by: Dan Bailey <danbailey@ilm.com>
cc713c6 to
056b322
Compare
|
@Idclip - I believe I've addressed all your feedback now. The primary goal here is to migrate the existing code off the tree/points API and into these codecs. As mentioned earlier, I'll try and further optimize performance in the new codecs. |
Signed-off-by: Dan Bailey <danbailey@ilm.com>
4076e47
into
AcademySoftwareFoundation:feature/io
2 participants
This adds the two codecs for handling point data - PointIndexCodec and PointDataCodec.
I have also addressed the feedback that was offered previously to rename MultiPass tag class to PointDataGridMultiPass and this now locks down the old multi-pass mechanism specific to only be usable by VDB Points.
Some other related changes include removing the out-of-core atomic in AttributeArray for
ABI=14and some minor re-organisation to be able to access thereadCompressedValues()/writeCompressedValues()specializations from the new codec.