Skip to content

G-API: Implement inference only mode for OV backend#24584

Merged
asmorkalov merged 2 commits intoopencv:4.xfrom
TolyaTalamanov:at/implement-inference-only-mode-for-ov-backend
Nov 29, 2023
Merged

G-API: Implement inference only mode for OV backend#24584
asmorkalov merged 2 commits intoopencv:4.xfrom
TolyaTalamanov:at/implement-inference-only-mode-for-ov-backend

Conversation

@TolyaTalamanov
Copy link
Copy Markdown
Contributor

@TolyaTalamanov TolyaTalamanov commented Nov 23, 2023

Changes overview

Introduced cv::gapi::wip::ov::benchmark_mode{} compile argument which if enabled force OpenVINO backend to run only inference without populating input and copying back output tensors.

This mode is only relevant for measuring the performance of pure inference without data transfers. Similar approach is using on OpenVINO side in benchmark_app: https://github.com/openvinotoolkit/openvino/blob/master/samples/cpp/benchmark_app/benchmark_app.hpp#L134-L139

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

@TolyaTalamanov
Copy link
Copy Markdown
Contributor Author

@dmatveev Could you have a look, please?

* This mode is used to evaluate the pure inference performance of the model without
* taking into account the i/o data transfer.
*/
struct inference_only { };
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  1. I'd move it to cv::gapi::wip
  2. I'd rename it to benchmark_mode to illustrate the purpose.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should it be cv::gapi::wip or cv::gapi::wip::ov?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the latter, of course.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

std::vector<cv::gimpl::GIslandExecutable::InObj> && input_objs,
std::vector<cv::gimpl::GIslandExecutable::OutObj> && output_objs);
std::vector<cv::gimpl::GIslandExecutable::OutObj> && output_objs,
const bool inference_only);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

probably you can omit it from there - as it is not the mandatory parameter.

it can be false internally by default.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is the place where inference_only from GOVExecutable is propogated to the specific inference kernel (e.g Infer, InferList, etc):
https://github.com/TolyaTalamanov/opencv/blob/at/implement-inference-only-mode-for-ov-backend/modules/gapi/src/backends/ov/govbackend.cpp#L1501-L1502

I guess there is no sense to do something like that:

auto ctx = std::make_shared<OVCallContext>(uu, out, op.args, op.outs,
            std::move(stub_meta), std::move(input_objs), std::move(output_objs));
if (m_inference_only) {
    ctx->enableInferenceOnly();
}

It's easier to pass this straight to context ctor.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It should remain in the class but it shouldn't be a mandatory constructor argument

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment on lines +1423 to +1424
m_inference_only =
cv::gapi::getCompileArg<cv::gapi::ov::inference_only>(compileArgs).has_value();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

probably you don't want to track all the individual fine-tuning knobs here, so I'd propose a configuration-like structure instead..

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Do you mean something like this?

namespace cv { namespace gapi { namespace wip { namespace { ov {
struct execution_config {
    bool enable_benchmark_mode = false;
};

} // ov
} // wip
} // gapi
} // cv

// Example:

auto compile_args = cv::compile_args(cv::gapi::wip::ov::execution_config{ true /* benchmark mode */ };

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not at the API level. Inside.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done


auto ctx = std::make_shared<OVCallContext>(uu, out, op.args, op.outs,
std::move(stub_meta), std::move(input_objs), std::move(output_objs));
std::move(stub_meta), std::move(input_objs), std::move(output_objs), m_inference_only);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

..and pass it as a whole thing here.

This method was quite generic, but now it knows explicitly about inference_only mode - purely an abstraction leak.

Copy link
Copy Markdown
Contributor Author

@TolyaTalamanov TolyaTalamanov Nov 24, 2023

Choose a reason for hiding this comment

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

This method still doesn't know about inference only implementation details it just propagates "some" configuration into the kernel. But your point is correct, need to decide who exactly should be responsible for handling this mode:

There are a few options:

  1. Kernel - This is the current approach. inference_only is propagated as the following:
    compileArgs -> GOVExecutable -> OVCallContext -> Kernel (e.g Infer, InferList, etc)

    The benefit is that IInferExecutor that used as proxy to submit inference tasks don't know about this option at all, the kernel hides this under set_input_data and read_output_data callbacks so it may continue work in sync/async modes without knowledges about this mode.

  2. RequestPool / IInferExecutor - In this apporach GOVExecutable may configure RequestPool the way it knows about benchmark mode.

Current approach looked less invasive.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes and my point is to make some configuration really a some configuration but not a specific field.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@TolyaTalamanov TolyaTalamanov force-pushed the at/implement-inference-only-mode-for-ov-backend branch from 3d2311d to 8e9f224 Compare November 27, 2023 12:43
Copy link
Copy Markdown
Contributor

@dmatveev dmatveev left a comment

Choose a reason for hiding this comment

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

👍 !

@dmatveev dmatveev self-assigned this Nov 29, 2023
@dmatveev dmatveev added this to the 4.9.0 milestone Nov 29, 2023
@TolyaTalamanov
Copy link
Copy Markdown
Contributor Author

@asmorkalov Can we merge it, please?

@asmorkalov asmorkalov merged commit 79797a3 into opencv:4.x Nov 29, 2023
IskXCr pushed a commit to Haosonn/opencv that referenced this pull request Dec 20, 2023
…rence-only-mode-for-ov-backend

G-API: Implement inference only mode for OV backend opencv#24584

### Changes overview

Introduced `cv::gapi::wip::ov::benchmark_mode{}` compile argument which if enabled force `OpenVINO` backend to run only inference without populating input and copying back output tensors. 

This mode is only relevant for measuring the performance of pure inference without data transfers. Similar approach is using on OpenVINO side in `benchmark_app`: https://github.com/openvinotoolkit/openvino/blob/master/samples/cpp/benchmark_app/benchmark_app.hpp#L134-L139



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

- [x] I agree to contribute to the project under Apache 2 License.
- [x] 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
- [x] 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.
- [x] The feature is well documented and sample code can be built with the project CMake
@asmorkalov asmorkalov mentioned this pull request Jan 19, 2024
thewoz pushed a commit to thewoz/opencv that referenced this pull request May 29, 2024
…rence-only-mode-for-ov-backend

G-API: Implement inference only mode for OV backend opencv#24584

### Changes overview

Introduced `cv::gapi::wip::ov::benchmark_mode{}` compile argument which if enabled force `OpenVINO` backend to run only inference without populating input and copying back output tensors. 

This mode is only relevant for measuring the performance of pure inference without data transfers. Similar approach is using on OpenVINO side in `benchmark_app`: https://github.com/openvinotoolkit/openvino/blob/master/samples/cpp/benchmark_app/benchmark_app.hpp#L134-L139



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

- [x] I agree to contribute to the project under Apache 2 License.
- [x] 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
- [x] 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.
- [x] The feature is well documented and sample code can be built with the project CMake
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.

3 participants