changed UMataData allocatorContext to OpenCLExecutionContext by kallaballa · Pull Request #24326 · opencv/opencv

kallaballa · 2023-09-27T07:16:32Z

… and bind it via scope everywhere needed.

Changing it from Context to OpenCLExecutionContext poses no problem, because everywhere I need the Context I can get it from the OpenCLExecutionContext.

The pertaining issue: #20486

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

I agree to contribute to the project under Apache 2 License.
To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
The PR is proposed to the proper branch
There is a reference to the original bug report and related work
There is accuracy test, performance test and test data in opencv_extra repository, if applicable
Patch to opencv_extra has the same branch name.
The feature is well documented and sample code can be built with the project CMake

force_builders=Win64 OpenCL,Linux OpenCL,Linux AVX2

… it via scope everywhere needed

kallaballa · 2023-09-27T07:41:26Z

Fixed a problem that was preventing UMat to UMat copy always

asmorkalov · 2023-10-04T07:12:00Z

@opencv-alalek could you review?

kallaballa · 2023-10-04T10:11:32Z

This seems odd: https://pullrequest.opencv.org/buildbot/builders/precommit_linux64/builds/104719/steps/test_core/logs/tests%20summary

opencv-alalek · 2023-10-04T15:19:45Z

To reproduce problem build locally with cmake ... -DWITH_OPENCL=OFF or run with OPENCV_OPENCL_RUNTIME=disabled

kallaballa · 2023-10-04T17:46:53Z

Oh my! Of course. please hold your horses. I'll fix that.

kallaballa · 2023-10-09T21:31:39Z

Doc build complains about trailing white space. gonna push a fix once this build has finished

kallaballa · 2023-10-11T05:53:41Z

Seeing the fails and will fix.

kallaballa · 2023-10-11T05:55:18Z

What is the best way to reproduce those cases? is the build configuration available?

opencv-alalek · 2023-10-11T06:50:05Z

Failed builds are with OpenCL (default). With / without OpenCL device available (both fails).

asmorkalov · 2023-10-12T08:29:54Z

/home/ci/opencv/modules/core/src/ocl.cpp:5826:62: warning: declaration of 'pExecCtx' shadows a previous local [-Wshadow]

/home/ci/opencv/modules/core/src/ocl.cpp:5827:50: warning: declaration of 'scope' shadows a previous local [-Wshadow]

/home/ci/opencv/modules/core/src/ocl.cpp:5835:62: warning: declaration of 'pExecCtx' shadows a previous local [-Wshadow]

/home/ci/opencv/modules/core/src/ocl.cpp:5836:50: warning: declaration of 'scope' shadows a previous local [-Wshadow]

kallaballa · 2023-10-12T08:30:42Z

/home/ci/opencv/modules/core/src/ocl.cpp:5826:62: warning: declaration of 'pExecCtx' shadows a previous local [-Wshadow]

/home/ci/opencv/modules/core/src/ocl.cpp:5827:50: warning: declaration of 'scope' shadows a previous local [-Wshadow]

/home/ci/opencv/modules/core/src/ocl.cpp:5835:62: warning: declaration of 'pExecCtx' shadows a previous local [-Wshadow]

/home/ci/opencv/modules/core/src/ocl.cpp:5836:50: warning: declaration of 'scope' shadows a previous local [-Wshadow]

Thanks! On it right now.

kallaballa · 2023-10-16T08:43:13Z

fixed: 784d86a

kallaballa · 2023-10-16T14:50:30Z

[ FAILED ] 16 tests, listed below:
[ FAILED ] Test_Caffe_layers.MVN/0, where GetParam() = OCV/OCL
[ FAILED ] Test_Caffe_layers.MVN/1, where GetParam() = OCV/OCL_FP16
[ FAILED ] Test_Caffe_layers.BatchNorm/0, where GetParam() = OCV/OCL
[ FAILED ] Test_Caffe_layers.BatchNorm/1, where GetParam() = OCV/OCL_FP16
[ FAILED ] Test_ONNX_layers.InstanceNorm/0, where GetParam() = OCV/OCL
[ FAILED ] Test_ONNX_layers.InstanceNorm/1, where GetParam() = OCV/OCL_FP16
[ FAILED ] Test_TensorFlow_layers.batch_norm_10/0, where GetParam() = OCV/OCL
[ FAILED ] Test_TensorFlow_layers.batch_norm_10/1, where GetParam() = OCV/OCL_FP16
[ FAILED ] Test_TensorFlow_layers.batch_norm_11/0, where GetParam() = OCV/OCL
[ FAILED ] Test_TensorFlow_layers.batch_norm_11/1, where GetParam() = OCV/OCL_FP16
[ FAILED ] Test_TensorFlow_layers.batch_norm_13/0, where GetParam() = OCV/OCL
[ FAILED ] Test_TensorFlow_layers.batch_norm_13/1, where GetParam() = OCV/OCL_FP16
[ FAILED ] Test_Torch_layers.run_batch_norm/0, where GetParam() = OCV/OCL
[ FAILED ] Test_Torch_layers.run_batch_norm/1, where GetParam() = OCV/OCL_FP16
[ FAILED ] Test_Torch_nets.FastNeuralStyle_accuracy/0, where GetParam() = OCV/OCL
[ FAILED ] Test_Torch_nets.FastNeuralStyle_accuracy/1, where GetParam() = OCV/OCL_FP16

That sounds serious. Best course of action is a Window 10 VM and building with tests?

kallaballa · 2023-10-16T14:55:04Z

~~But there seems something wrong with the tests:~~

~~test_dnn-ippicv-opencl test_dnn-ippicv-opencl ; cases (tests): 85 (8069) ; passed: 8053 ; skipped: 23 ; failed~~

It's the same issue.

opencv-alalek

Please add tests for related use cases here: https://github.com/opencv/opencv/blob/4.x/modules/core/test/test_opencl.cpp

opencv-alalek · 2023-10-18T20:32:23Z

modules/core/src/ocl.cpp

    if( arg.m )
    {
+        std::shared_ptr<ocl::OpenCLExecutionContext> pExecCtx = std::static_pointer_cast<ocl::OpenCLExecutionContext>(arg.m->u->allocatorContext);
+        OpenCLExecutionContextScope scope(pExecCtx ? *pExecCtx.get() : ocl::OpenCLExecutionContext::getCurrent());


Kernel::

OpenCLExecutionContextScope

We should not change the current context selected by caller.

We need to ensure that:

kernel is from the current (default) OpenCLExecutionContext (or assume that)

and all passed buffers are compatible with the current (default) OpenCLExecutionContext (the same "OpenCL context")

If it is not true, then raise an error.

How to properly compare to OpenCLExecutionContexts? I'd write the operator, but I don't know what to compare.

"OpenCL context" means handle of type cl_context.

E.g. compare these values related to OpenCLExecutionContexts: execCtx.getContext().ptr()

opencv-alalek · 2023-10-18T20:49:01Z

modules/core/src/umatrix.cpp

+#ifdef HAVE_OPENCL
+        std::shared_ptr<ocl::OpenCLExecutionContext> pExecCtxSrc = std::static_pointer_cast<ocl::OpenCLExecutionContext>(u->allocatorContext);
+        std::shared_ptr<ocl::OpenCLExecutionContext> pExecCtxDst = std::static_pointer_cast<ocl::OpenCLExecutionContext>(dst.u->allocatorContext);
+        if ((pExecCtxSrc && pExecCtxDst && pExecCtxSrc->getContext().ptr() == pExecCtxDst->getContext().ptr()) && u->currAllocator == dst.u->currAllocator)


dst should belong to the current context.

if src is not related to the current context then we should force OpenCLExecutionContextScope below

Mat dst = _dst.getMat(); { OpenCLExecutionContextScope ... u->currAllocator->download(u, dst.ptr(), dims, sz, srcofs, step.p, dst.step.p); }

opencv-alalek · 2023-10-18T20:51:51Z

modules/core/src/umatrix.cpp

                             haveDstUninit ? " -D HAVE_DST_UNINIT" : "");

+        std::shared_ptr<ocl::OpenCLExecutionContext> pExecCtx = std::static_pointer_cast<ocl::OpenCLExecutionContext>(u->allocatorContext);
+        ocl::OpenCLExecutionContextScope scope(pExecCtx ? *pExecCtx.get() : ocl::OpenCLExecutionContext::getCurrent());


OpenCLExecutionContextScope

We should not change execution scope here.

If src's scope is not the same (as dst/thread), then .clone() src UMat first.
If mask's scope is not the same (as as dst/thread), then .clone() mask UMat first.

Done, and i used the same solution for convertTo

asmorkalov · 2023-11-08T13:32:14Z

@kallaballa Friendly reminder.

asmorkalov · 2023-11-16T12:12:42Z

@kallaballa Friendly reminder.

…convertTo clones UMats if they don't match the current context.

kallaballa · 2023-11-20T17:24:04Z

Waiting for the tests. Apart from that i think the PR is ready.

kallaballa · 2023-11-21T06:57:32Z

Local tests failed so i made a new patch. now local tests succeed.

kallaballa · 2023-11-21T07:11:01Z

found more cases that slipped by me. local tests succeed. waiting for the CI.

kallaballa · 2023-11-21T13:23:11Z

The failing tests look suspicious and i will look into it. anyway it did some testing using my demos of V4D (on a unpublished branch) and I found several problems that prevented me from using UMat with multiple OpenCL contexts.

I will add another commit that was necessary to fix the issues i had but I am overall not sure I got this right in the first place. Maybe I misunderstood? @opencv-alalek could you have a second look on my approach to this?

Also. I currently do the following to copy cross cl-context:

CV_EXPORTS void copy_cross(const cv::UMat& src, cv::UMat& dst) {
	if(dst.empty())
		dst.create(src.size(), src.type());
	src.copyTo(dst.getMat(cv::ACCESS_READ));
}

Is that the right/recommended way? I tried several other variants but to no avail.

asmorkalov · 2023-12-20T12:07:10Z

@kallaballa @opencv-alalek What is the PR status?

opencv-alalek

Thank you for the update!

Could you prepare test cases which cover new code paths?

E.g, here: https://github.com/opencv/opencv/blob/4.x/modules/core/test/test_opencl.cpp (contains tests for OpenCLExecutionContext)

opencv-alalek · 2023-11-24T02:52:31Z

modules/core/src/ocl.cpp

@@ -1,4 +1,4 @@
-/*M///////////////////////////////////////////////////////////////////////////////////////
+    /*M///////////////////////////////////////////////////////////////////////////////////////


unnecessary change

opencv-alalek · 2023-11-24T03:01:15Z

modules/core/src/umatrix.cpp

+                (
+                        (currentExecCtx.empty() && !pExecCtxDst->empty())
+                        || (!currentExecCtx.empty() && pExecCtxDst->empty())
+                        || (pExecCtxDst->getContext().ptr() != currentExecCtx.getContext().ptr())


It makes sense to add method + test (for such method) instead of writing complex conditions multiple times.

// class OpenCLExecutionContext in ocl.hpp bool isCompatibleContext(const std::shared_ptr<ocl::OpenCLExecutionContext>& other) { ... }

Usage:

if (pExecCtxDst && !pExecCtxDst->isCompatibleContext(pExecCtxSrc)) CV_Error(...)

kallaballa · 2025-01-03T23:16:24Z

This is a prototype that needs rework mostly to improve code-reuse and clarity. That said it does fix the problem and considerably improves performance by avoiding errors and copies (whenever possible) on then many simple and complex tested workloads.

kallaballa · 2025-01-03T23:19:51Z

Also when switching between 4.x and this branch i can't find unwanted side effects.

kallaballa · 2025-12-06T05:16:37Z

should i pursue this one? i know it looks very ugly but the gains are real.
}

kallaballa added 2 commits September 27, 2023 09:11

changed UMataData allocatorContext to OpenCLExecutionContext and bind…

d1eeabc

… it via scope everywhere needed

use the right comparision to determine matching contexts

c70cae9

opencv-alalek added the category: ocl label Sep 29, 2023

asmorkalov requested a review from opencv-alalek October 4, 2023 07:11

asmorkalov added this to the 4.9.0 milestone Oct 5, 2023

guard opencl specific code in UMat::copy

c741080

white space fixes

d8900de

kallaballa added 2 commits October 12, 2023 08:19

guard against null openclexecutioncontext

971893d

more guards against null openclexecutioncontext

e6a8171

avoid shadowing warning

784d86a

opencv-alalek reviewed Oct 18, 2023

View reviewed changes

OpenCL: Kernel::set throws an error on diverging contexts and copyTo/…

7453b0a

…convertTo clones UMats if they don't match the current context.

kallaballa mentioned this pull request Nov 20, 2023

fix race condition in oclCvtColorBGR2HSV (color_hsv.dispatch.cpp) by using a mutex #24444

Closed

6 tasks

allow for nullptr allocatorContext

6d09d54

don't for context scope anywhre

1225291

only try a GPU copy if all the contexts match

ba2822a

kallaballa added 2 commits November 21, 2023 14:24

more cases where special CL context handling is required.

c1535fd

removed redundant cl exec scope

733f9e0

asmorkalov modified the milestones: 4.9.0, 4.10.0 Dec 20, 2023

opencv-alalek added the pr: needs test New functionality requires minimal tests set label Dec 20, 2023

opencv-alalek reviewed Dec 20, 2023

View reviewed changes

opencv-alalek added the feature label Dec 20, 2023

asmorkalov modified the milestones: 4.10.0, 4.11.0 May 16, 2024

asmorkalov modified the milestones: 4.11.0, 4.12.0 Dec 20, 2024

kallaballa added 2 commits January 4, 2025 00:12

merge

9d85c31

Merge branch 'opencv-4.x' into fix_diverging_cl_contexts

16cb3d1

asmorkalov modified the milestones: 4.12.0, 4.13.0 May 5, 2025

asmorkalov added 2 commits November 17, 2025 13:50

Core review fixes.

b69fee6

Merge branch '4.x' into fix_diverging_cl_contexts

9271727

asmorkalov self-requested a review November 17, 2025 10:51

asmorkalov modified the milestones: 4.13.0, 4.14.0 Dec 26, 2025

		@@ -1,4 +1,4 @@
		/*M///////////////////////////////////////////////////////////////////////////////////////
		/*M///////////////////////////////////////////////////////////////////////////////////////

Uh oh!

Conversation

kallaballa commented Sep 27, 2023 • edited by opencv-alalek Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Readiness Checklist

Uh oh!

kallaballa commented Sep 27, 2023

Uh oh!

asmorkalov commented Oct 4, 2023

Uh oh!

kallaballa commented Oct 4, 2023

Uh oh!

opencv-alalek commented Oct 4, 2023

Uh oh!

kallaballa commented Oct 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kallaballa commented Oct 9, 2023

Uh oh!

kallaballa commented Oct 11, 2023

Uh oh!

kallaballa commented Oct 11, 2023

Uh oh!

opencv-alalek commented Oct 11, 2023

Uh oh!

asmorkalov commented Oct 12, 2023

Uh oh!

kallaballa commented Oct 12, 2023

Uh oh!

kallaballa commented Oct 16, 2023

Uh oh!

kallaballa commented Oct 16, 2023

Uh oh!

kallaballa commented Oct 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

opencv-alalek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asmorkalov commented Nov 8, 2023

Uh oh!

asmorkalov commented Nov 16, 2023

Uh oh!

kallaballa commented Nov 20, 2023

Uh oh!

kallaballa commented Nov 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kallaballa commented Nov 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kallaballa commented Nov 21, 2023

Uh oh!

asmorkalov commented Dec 20, 2023

Uh oh!

opencv-alalek left a comment

Choose a reason for hiding this comment

Uh oh!

kallaballa commented Sep 27, 2023 •

edited by opencv-alalek

Loading

kallaballa commented Oct 4, 2023 •

edited

Loading

kallaballa commented Oct 16, 2023 •

edited

Loading

kallaballa commented Nov 21, 2023 •

edited

Loading

kallaballa commented Nov 21, 2023 •

edited

Loading

kallaballa commented Jan 3, 2025 •

edited

Loading