Content-Length: 360557 | pFad | https://github.com/opencv/opencv/pull/24844

66 Make use of C++11 std::aligned_alloc where available by georgthegreat · Pull Request #24844 · opencv/opencv · GitHub
Skip to content

Make use of C++11 std::aligned_alloc where available #24844

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

Closed
wants to merge 1 commit into from

Conversation

georgthegreat
Copy link
Contributor

This is more like a food for thought, not a final state ready to be merged.

As I see, opencv uses constexpr statement and thus should mandate C++11 to be built.
C++ introduces much more compatible way for allocating aligned memory blocks.

Maybe opencv can make use of it too?

return ptr;
}
#elif defined HAVE_WIN32_ALIGNED_MALLOC
if (isAlignedAllocationEnabled())
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wornder if runtime / build time switcher for disabling aligned allocation are still relevant

return ptr;
}
#if defined(_MSC_VER)
void* ptr = _aligned_alloc(size, CV_MALLOC_ALIGN);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Windows does not have std::aligned_alloc as their free() method does not support freeing such blocks.

I will add comment regarding this limitation it a further commits.

@s-trinh
Copy link
Contributor

s-trinh commented Jan 13, 2024

std::aligned_alloc needs C++17: https://en.cppreference.com/w/cpp/memory/c/aligned_alloc

@georgthegreat
Copy link
Contributor Author

Whoops! I assume we can not mandate such function, can we?

@s-trinh
Copy link
Contributor

s-trinh commented Jan 14, 2024

OpenCV 4 requires >= C++11
I think OpenCV 5 requires >= C++17 ?

Looks like there are already plans to use std::aligned_alloc():

opencv/CMakeLists.txt

Lines 761 to 769 in 2791bb7

if(OPENCV_ENABLE_MEMALIGN)
CHECK_SYMBOL_EXISTS(posix_memalign stdlib.h HAVE_POSIX_MEMALIGN)
CHECK_INCLUDE_FILE(malloc.h HAVE_MALLOC_H)
if(HAVE_MALLOC_H)
CHECK_SYMBOL_EXISTS(memalign malloc.h HAVE_MEMALIGN)
endif()
# TODO:
# - std::aligned_alloc() C++17 / C11
endif()


From a quick search, I did not find the minimal requirements (C++/C versions, target architectures, CUDA version/support, Python version/support, ...) for the different main OpenCV versions?

I think it would be great to add these info on these pages?

@asmorkalov
Copy link
Contributor

@georgthegreat Thanks a lot for the contribution. The PR was discussed on OpenCV Core team meeting. We propose not to remove the existing branches, but wrap the new standard branch with C++ version guard and put it on the first priority. It means:

  • No/less collisions in 4.x->5.x merge
  • The standard branch is used first, if available

@georgthegreat
Copy link
Contributor Author

@asmorkalov, thanks for carrying out the negotiation.

I will bring the PR later on, but, first, what do you think about removing runtime checks for isAlignedAllocationEnabled()?

@asmorkalov
Copy link
Contributor

/cc @opencv-alalek @vpisarev

@asmorkalov
Copy link
Contributor

Both C11 and C++17 of aligned_alloc requires: "Passing a size which is not an integral multiple of alignment or an alignment which is not valid or not supported by the implementation causes the function to fail and return a null pointer (C11, as published, specified undefined behavior in this case, this was corrected by DR460)."

Proof:

Current OpenCV implementation does not follow the rule when the fastMalloc is called. It leads to the following test failures:

[ RUN      ] OCL_MeanStdDev_.ZeroMask
CV_MALLOC_ALIGN: 64
unknown file: Failure
C++ exception with description "OpenCV(4.12.0-dev) /mnt/Projects/Projects/opencv/modules/core/src/alloc.cpp:72: error: (-4:Insufficient memory) Failed to allocate 100 bytes in function 'OutOfMemoryError'
" thrown in the test body.
[  FAILED  ] OCL_MeanStdDev_.ZeroMask (3 ms)
[----------] 1 test from OCL_MeanStdDev_ (3 ms total)

[----------] 1 test from OCL_Gemm
[ RUN      ] OCL_Gemm.small
CV_MALLOC_ALIGN: 64
unknown file: Failure
C++ exception with description "OpenCV(4.12.0-dev) /mnt/Projects/Projects/opencv/modules/core/src/alloc.cpp:72: error: (-4:Insufficient memory) Failed to allocate 24 bytes in function 'OutOfMemoryError'
" thrown in the test body.
[  FAILED  ] OCL_Gemm.small (0 ms)

@asmorkalov asmorkalov closed this Mar 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: https://github.com/opencv/opencv/pull/24844

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy