-
Notifications
You must be signed in to change notification settings - Fork 5
fix: Complete memory management with dynamic optimization supportFix/memory problems #10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,8 +2,52 @@ | |||||
| #include "sam3.cuh" | ||||||
| #include <chrono> | ||||||
| #include <thread> | ||||||
| #include <memory> | ||||||
| #include <opencv2/imgproc.hpp> | ||||||
|
|
||||||
| void ensure_even_dimensions(const cv::Mat& input, cv::Mat& output) | ||||||
| { | ||||||
| int new_width = input.cols; | ||||||
| int new_height = input.rows; | ||||||
| bool needs_resize = false; | ||||||
|
|
||||||
| if (input.cols % 2 != 0) | ||||||
| { | ||||||
| new_width = input.cols + 1; | ||||||
| needs_resize = true; | ||||||
| } | ||||||
|
|
||||||
| if (input.rows % 2 != 0) | ||||||
| { | ||||||
| new_height = input.rows + 1; | ||||||
| needs_resize = true; | ||||||
| } | ||||||
|
|
||||||
| if (needs_resize) | ||||||
| { | ||||||
| cv::resize(input, output, cv::Size(new_width, new_height), 0, 0, cv::INTER_LINEAR); | ||||||
| } | ||||||
| else | ||||||
| { | ||||||
| output = input; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| cv::Mat read_and_ensure_even(const std::string imgpath) | ||||||
| { | ||||||
| cv::Mat img_original = cv::imread(imgpath, cv::IMREAD_COLOR); | ||||||
| if (img_original.empty()) | ||||||
| { | ||||||
| std::stringstream err; | ||||||
| err << "Failed to read image: " << imgpath; | ||||||
| throw std::runtime_error(err.str()); | ||||||
| } | ||||||
|
|
||||||
| cv::Mat img; | ||||||
| ensure_even_dimensions(img_original, img); | ||||||
| return img; | ||||||
| } | ||||||
|
|
||||||
| void read_image_into_buffer(const std::string imgpath, char* raw_buffer, cv::Mat& buffer) | ||||||
| { | ||||||
| size_t file_size = std::filesystem::file_size(imgpath); | ||||||
|
|
@@ -90,36 +134,47 @@ int main(int argc, char* argv[]) | |||||
|
|
||||||
| const float vis_alpha = 0.3; | ||||||
| const float probability_threshold = 0.5; | ||||||
| const SAM3_VISUALIZATION visualize = SAM3_VISUALIZATION::VIS_INSTANCE_SEGMENTATION; | ||||||
| const SAM3_VISUALIZATION visualize = SAM3_VISUALIZATION::VIS_SEMANTIC_SEGMENTATION; | ||||||
|
||||||
| const SAM3_VISUALIZATION visualize = SAM3_VISUALIZATION::VIS_SEMANTIC_SEGMENTATION; | |
| const SAM3_VISUALIZATION visualize = SAM3_VISUALIZATION::VIS_INSTANCE_SEGMENTATION; |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,6 +4,11 @@ SAM3_PCS::SAM3_PCS(const std::string engine_path, const float vis_alpha, const f | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : _engine_path(engine_path) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| , _overlay_alpha(vis_alpha) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| , _probability_threshold(prob_threshold) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| , opencv_input(nullptr) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| , gpu_result(nullptr) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| , zc_input(nullptr) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| , gpu_colpal(nullptr) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| , opencv_inbytes(0) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cuda_check(cudaStreamCreate(&sam3_stream), "creating CUDA stream for SAM3"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -21,6 +26,47 @@ SAM3_PCS::SAM3_PCS(const std::string engine_path, const float vis_alpha, const f | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| bsize.y=16; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| std::pair<cv::Mat, std::shared_ptr<void>> SAM3_PCS::allocate_pinned_mat(int rows, int cols, int type) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| size_t bytes = rows * cols * CV_ELEM_SIZE(type); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| void* ptr = nullptr; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cuda_check(cudaMallocHost(&ptr, bytes), " allocating pinned memory for Mat"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| auto deleter = [](void* p) { if (p) cudaFreeHost(p); }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| std::shared_ptr<void> mem_holder(ptr, deleter); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cv::Mat mat(rows, cols, type, ptr); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return std::make_pair(mat, mem_holder); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| void SAM3_PCS::setup_pinned_matrices(cv::Mat& input_mat, cv::Mat& result_mat) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| opencv_inbytes = input_mat.total() * input_mat.elemSize(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (is_zerocopy) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cuda_check(cudaHostGetDevicePointer(&zc_input, input_mat.data, 0), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| " getting GPU pointer for pinned input Mat"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cuda_check(cudaHostGetDevicePointer(&gpu_result, result_mat.data, 0), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| " getting GPU pointer for pinned result Mat"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (opencv_input != nullptr) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cudaFree(opencv_input); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cudaFree((void*)gpu_result); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+60
to
+61
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cudaFree(opencv_input); | |
| cudaFree((void*)gpu_result); | |
| cuda_check(cudaFree(opencv_input), " freeing opencv input memory on a dGPU system"); | |
| cuda_check(cudaFree((void*)gpu_result), " freeing result memory on a dGPU system"); |
Copilot
AI
Jan 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a potential memory leak and state inconsistency issue in the reallocation logic. If cudaMalloc for opencv_input succeeds (line 66) but cudaMalloc for gpu_result fails (line 67), the pointers will be in an inconsistent state. The old memory is already freed, opencv_inbytes is updated, but gpu_result is nullptr while opencv_input points to newly allocated memory. Consider setting pointers to nullptr after freeing, or using a temporary variable pattern to ensure atomicity of the reallocation.
| if (opencv_input != nullptr) | |
| { | |
| cudaFree(opencv_input); | |
| cudaFree((void*)gpu_result); | |
| } | |
| cuda_check(cudaMalloc(&opencv_input, opencv_inbytes), " allocating opencv input memory on a dGPU system"); | |
| cuda_check(cudaMalloc((void**)&gpu_result, opencv_inbytes), " allocating result memory on a dGPU system"); | |
| // Use temporary pointers so reallocation is atomic and leaves the object in a consistent state | |
| void* new_opencv_input = nullptr; | |
| void* new_gpu_result = nullptr; | |
| // Allocate new buffers first; cuda_check will handle any allocation errors | |
| cuda_check(cudaMalloc(&new_opencv_input, opencv_inbytes), " allocating opencv input memory on a dGPU system"); | |
| cuda_check(cudaMalloc(&new_gpu_result, opencv_inbytes), " allocating result memory on a dGPU system"); | |
| // Now that both allocations have succeeded, free any old buffers | |
| if (opencv_input != nullptr) | |
| { | |
| cudaFree(opencv_input); | |
| cudaFree((void*)gpu_result); | |
| } | |
| // Update member pointers to point to the newly allocated buffers | |
| opencv_input = new_opencv_input; | |
| gpu_result = new_gpu_result; | |
| // Initialize the buffers |
Copilot
AI
Jan 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cudaMemset calls should be wrapped with cuda_check for consistent error handling. If these operations fail, it could lead to undefined behavior when the uninitialized memory is used.
| cudaMemset(opencv_input, 0, opencv_inbytes); | |
| cudaMemset((void *)gpu_result, 0, opencv_inbytes); | |
| cuda_check(cudaMemset(opencv_input, 0, opencv_inbytes), " zeroing opencv input memory on a dGPU system"); | |
| cuda_check(cudaMemset((void *)gpu_result, 0, opencv_inbytes), " zeroing result memory on a dGPU system"); |
Copilot
AI
Jan 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cudaFree calls should be wrapped with cuda_check for consistent error handling. While cudaFree typically doesn't fail for valid pointers, it's a best practice to check for errors consistently.
| cudaFree(opencv_input); | |
| cudaFree((void*)gpu_result); | |
| cuda_check(cudaFree(opencv_input), " freeing opencv input memory"); | |
| cuda_check(cudaFree((void*)gpu_result), " freeing result memory"); |
Copilot
AI
Jan 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a potential memory leak and state inconsistency issue in the reallocation logic. If cudaMalloc for opencv_input succeeds (line 219) but cudaMalloc for gpu_result fails (line 220), the pointers will be in an inconsistent state. The old memory is already freed, opencv_inbytes is updated, but gpu_result is nullptr while opencv_input points to newly allocated memory. Consider setting pointers to nullptr after freeing, or using a temporary variable pattern to ensure atomicity of the reallocation.
| if (opencv_input != nullptr) | |
| { | |
| cudaFree(opencv_input); | |
| cudaFree((void*)gpu_result); | |
| } | |
| opencv_inbytes = current_inbytes; | |
| cuda_check(cudaMalloc(&opencv_input, opencv_inbytes), " reallocating opencv input memory"); | |
| cuda_check(cudaMalloc((void**)&gpu_result, opencv_inbytes), " reallocating result memory"); | |
| // Allocate new buffers first, using temporaries to keep state consistent | |
| void* new_opencv_input = nullptr; | |
| void* new_gpu_result = nullptr; | |
| cuda_check(cudaMalloc(&new_opencv_input, current_inbytes), " reallocating opencv input memory"); | |
| cuda_check(cudaMalloc(&new_gpu_result, current_inbytes), " reallocating result memory"); | |
| // Free old buffers after successful allocations | |
| if (opencv_input != nullptr) | |
| { | |
| cudaFree(opencv_input); | |
| opencv_input = nullptr; | |
| } | |
| if (gpu_result != nullptr) | |
| { | |
| cudaFree((void*)gpu_result); | |
| gpu_result = nullptr; | |
| } | |
| // Commit new state atomically | |
| opencv_input = new_opencv_input; | |
| gpu_result = static_cast<decltype(gpu_result)>(new_gpu_result); | |
| opencv_inbytes = current_inbytes; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PinnedMemoryDeleter struct is defined but never used in the codebase. The allocate_pinned_mat function uses an inline lambda deleter instead. Consider either using this struct in allocate_pinned_mat or removing it to reduce code clutter.