[BUG] multibyte_split implicitly synchronizes all streams

[vyasr]

Describe the bug
Currently multibyte_split uses a stream pool to divide up work between multiple streams. However, output_builder.inplace_resize still resizes an rmm::device_uvector on CUDA's default stream. This function is being called from output_builder.advance_output, which does not currently accept a stream. This is problematic since the default stream implicitly synchronizes with all streams.

Expected behavior
From a quick analysis it is not clear to me whether advance_output or inplace_resize should be running on the scan_stream or the read_stream inside multibyte_split. Based on where advance_output is called, I suspect that it actually requires an explicit synchronization between these two streams, and perhaps the existing events are already intended to solve that need. Fixing this would require modifying this function to run on the appropriate stream, and perhaps introducing an explicit synchronization or some appropriate event waiting.

[vyasr]

Perhaps @upsj or @cwharris have some insight into the appropriate fix here.

[upsj]

Does this actually synchronize? device_uvector doesn't and device_buffer doesn't either in the shrinking case, as far as I can tell. It only modifies the size member appropriately.
I have a choice between two approaches: Use device_uvector as a fixed-sized buffer and store its actual size separately, or use size and capacity to keep track of how many elements inside the vector are already in use. What do you think - should this be marked/documented more explicitly or store the size separately?
Without the sync-less resize, having a separate capacity and size seems unnecessary to me, and the reserve method no longer has a valid use case.

[vyasr]

Does this actually synchronize? device_uvector doesn't and device_buffer doesn't either in the shrinking case, as far as I can tell. It only modifies the size member appropriately.

You're right that neither of those synchronizes. However, device_buffer::resize sets the stream used for deallocating the buffer, which ends up triggering a sync on that stream when the destructor of the uvector is called. Both char_storage and row_offset_storage are function scoped, so they are only destroyed at the end of the function when this function's frame is popped off the stack. join_streams is called well before that and it looks like the input stream is set to wait on both the streams in the pool, so I think all that needs to happen is that the input stream needs to get passed through advance_output to inplace_resize so that the vectors in the two output_builders (char_storage and row_offset_storage) are destroyed on the user-provided input stream, i.e. after all work in multibyte split is done but without using the default stream. Avoiding the default stream saves us triggering a sync at the end of multibyte_split, which will be undesirable once we expose a truly stream-ordered API that allows users to run multiple libcudf APIs (or other external functions) simultaneously on different streams.

[upsj]

Thanks for the details, I'll adopt your suggestions

[vyasr]

@upsj in the interest of making sure everything works together I saved you the time and have gone ahead and implemented my proposed solution in #11875. Tests still pass for me locally, so I think it should be OK, but I'll keep you apprised if something goes wrong!