Conversation
Signed-off-by: ktro2828 <kotaro.uetake@tier4.jp>
There was a problem hiding this comment.
Pull Request Overview
This PR refactors configuration and preprocessing logic for 2D detector and segmenter classes to improve performance by avoiding repeated device memory allocations. The main optimization moves image normalization parameters (mean and std) to device memory once during configuration construction instead of reallocating them on every preprocessing call.
- Refactored all config structs to upload
meanandstdarrays to device memory in constructors - Updated constructors to use move semantics for better resource management
- Simplified preprocessing methods to use pre-allocated device memory directly
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| mmros/include/mmros/detector/detector2d.hpp | Added constructor to Detector2dConfig for device memory allocation and updated class constructor signature |
| mmros/include/mmros/detector/instance_segmenter2d.hpp | Added constructor to InstanceSegmenter2dConfig for device memory allocation and updated class constructor signature |
| mmros/include/mmros/detector/panoptic_segmenter2d.hpp | Added constructor to PanopticSegmenter2dConfig for device memory allocation and updated class constructor signature |
| mmros/include/mmros/detector/semantic_segmenter2d.hpp | Added constructor to SemanticSegmenter2dConfig for device memory allocation and updated class constructor signature |
| mmros/src/detector/detector2d.cpp | Updated constructor to use move semantics and simplified preprocessing to use device-allocated arrays |
| mmros/src/detector/instance_segmeter2d.cpp | Updated constructor to use move semantics and simplified preprocessing to use device-allocated arrays |
| mmros/src/detector/panoptic_segmenter2d.cpp | Updated constructor to use move semantics and simplified preprocessing to use device-allocated arrays |
| mmros/src/detector/semantic_segmenter2d.cpp | Updated constructor to use move semantics and simplified preprocessing to use device-allocated arrays |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| Detector2dConfig( | ||
| const std::vector<double> & _mean, const std::vector<double> & _std, | ||
| archetype::BoxFormat2D _box_format, double _score_threshold) | ||
| : box_format(_box_format), score_threshold(_score_threshold) |
There was a problem hiding this comment.
The constructor should validate that the mean and std vectors have the same size and are not empty before allocating device memory. Consider adding size validation to prevent runtime errors.
| InstanceSegmenter2dConfig( | ||
| const std::vector<double> & _mean, const std::vector<double> & _std, | ||
| archetype::BoxFormat2D _box_format, double _score_threshold) | ||
| : box_format(_box_format), score_threshold(_score_threshold) |
There was a problem hiding this comment.
The constructor should validate that the mean and std vectors have the same size and are not empty before allocating device memory. Consider adding size validation to prevent runtime errors.
| PanopticSegmenter2dConfig( | ||
| const std::vector<double> & _mean, const std::vector<double> & _std, | ||
| archetype::BoxFormat2D _box_format, double _score_threshold) | ||
| : box_format(_box_format), score_threshold(_score_threshold) |
There was a problem hiding this comment.
The constructor should validate that the mean and std vectors have the same size and are not empty before allocating device memory. Consider adding size validation to prevent runtime errors.
| { | ||
| std::vector<double> mean; //!< Image mean. | ||
| std::vector<double> std; //!< Image std. | ||
| SemanticSegmenter2dConfig(const std::vector<double> & _mean, const std::vector<double> & _std) |
There was a problem hiding this comment.
The constructor should validate that the mean and std vectors have the same size and are not empty before allocating device memory. Consider adding size validation to prevent runtime errors.
Description
This pull request refactors the configuration and preprocessing logic for all 2D detector and segmenter classes to improve efficiency and clarity. The main change is moving the image normalization parameters (
meanandstd) to device memory once during configuration construction, rather than reloading and copying them on every preprocessing call. Additionally, constructors now use move semantics for configuration objects, ensuring better resource management.Configuration and Device Memory Improvements:
Detector2dConfig,InstanceSegmenter2dConfig,PanopticSegmenter2dConfig,SemanticSegmenter2dConfig) to uploadmeanandstdarrays to device memory in their constructors, replacing host-side vectors with device pointers (CudaUniquePtr<float[]>). This avoids repeated device memory allocations and copies during preprocessing. [1] [2] [3] [4]Constructor and Resource Management Updates:
&&), and store them usingstd::move, ensuring efficient resource transfer and ownership. [1] [2] [3] [4] [5] [6] [7] [8]Preprocessing Efficiency:
preprocessmethods in all detector and segmenter classes to use the device-sidemeanandstdarrays directly, removing redundant host-to-device memory operations and related temporary allocations. [1] [2] [3] [4]How was this PR tested?
yolox,deimv2,pidnet,eomtNotes for reviewers
None.
Effects on system behavior
None.