Multiprocessing for frame encoding in Segmentation constructor #245
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
hackermd
reviewed
Aug 5, 2023
src/highdicom/seg/sop.py
Outdated
| segment_array[plane_index].flatten() | ||
| ) | ||
| # Concatenate the 1D array for encoding at the end | ||
| full_frames_list.append(segment_array.flatten()) |
There was a problem hiding this comment.
I wonder whether a separate variable is needed or whether frames (compressed or not) could just be stored in a list called frames? How the list of frames then needs to be handled (encapsulation or native encoding) is determined by is_enaps.
The variable for the futures could be called frames_futures.
In my opinion, that would make it easier to follow.
There was a problem hiding this comment.
I did it this way primarily to appease mypy, but I guess it probably hurts readability. Implemented 5c5cbb6 as you suggest
|
Yes I have definitely thought about decoding with multiple workers too, one for the to-do list. |
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.
Add the ability to use concurrency to encode frames when creating segmentations.
When using encapsulated transfer syntaxes, the process of encoding frames takes a significant amount of the total time for creation of a Segmentation object. For large multiframe segmentations such as those of WSIs, this can take several minutes. Parallelising the encoding of frames is an obvious place for significant performance gains.
In this PR, I add the ability to use multiprocessing for frame encoding using the Python standard library's
concurrent.futuresmodule. The user specifies aworkersparameter governing the number of worker processes to spawn (with the default remaining no worker threads). Alternatively they may "bring their own" worker pool by providing an instance of anyconcurrent.futures.Executor. This has two potential benefits. First it allows a worker pool to be re-used in programs where a large number of segmentations needs to be created to save to overhead of setting up the worker pools. Secondly, it gives the user more control over the multiprocessing context if they want it. For example, in principle, they could pass a ThreadPoolExecutor to use multi-threading rather that multiprocessing.I also made a few other tweaks to the segmentation array processing for further efficiency gains. Most notably, I adjusted the
_get_segment_pixel_arraymethod to return the array for a single frame and a single segment (as opposed to a all frames and a single segment). This makes the looping logic a bit simpler and it turns out that this is slightly faster, presumably because it avoid the need to allocate memory for large arrays.