G-API: Implement inference only mode for OV backend#24584
Conversation
|
@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 { }; |
There was a problem hiding this comment.
- I'd move it to
cv::gapi::wip - I'd rename it to
benchmark_modeto illustrate the purpose.
There was a problem hiding this comment.
Should it be cv::gapi::wip or cv::gapi::wip::ov?
| 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); |
There was a problem hiding this comment.
probably you can omit it from there - as it is not the mandatory parameter.
it can be false internally by default.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
It should remain in the class but it shouldn't be a mandatory constructor argument
| m_inference_only = | ||
| cv::gapi::getCompileArg<cv::gapi::ov::inference_only>(compileArgs).has_value(); |
There was a problem hiding this comment.
probably you don't want to track all the individual fine-tuning knobs here, so I'd propose a configuration-like structure instead..
There was a problem hiding this comment.
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 */ };
There was a problem hiding this comment.
Not at the API level. Inside.
|
|
||
| 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); |
There was a problem hiding this comment.
..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.
There was a problem hiding this comment.
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:
-
Kernel- This is the current approach.inference_onlyis propagated as the following:
compileArgs->GOVExecutable->OVCallContext->Kernel(e.gInfer,InferList, etc)The benefit is that
IInferExecutorthat used as proxy to submit inference tasks don't know about this option at all, thekernelhides this underset_input_dataandread_output_datacallbacks so it may continue work insync/asyncmodes without knowledges about this mode. -
RequestPool / IInferExecutor- In this apporachGOVExecutablemay configureRequestPoolthe way it knows about benchmark mode.
- Prons:
- Kernels don't know about this mode so they continue submit their tasks through
RequestPoolAPI.
- Kernels don't know about this mode so they continue submit their tasks through
- Cons:
inference_onlymode will be enabled for allKernel's (now it's only relevant forInfer(InferList,InferROI,InferList2throw exception if mode is enabled).- Mode should be handled for both
SyncInferExecutorandAsyncInferExecutor. This is a little bit tricky since currentlyread_output_datais also responsible for posting outputs to maintain contract with streaming executor.
https://github.com/TolyaTalamanov/opencv/blob/at/implement-inference-only-mode-for-ov-backend/modules/gapi/src/backends/ov/govbackend.cpp#L471
Current approach looked less invasive.
There was a problem hiding this comment.
Yes and my point is to make some configuration really a some configuration but not a specific field.
3d2311d to
8e9f224
Compare
|
@asmorkalov Can we merge it, please? |
…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
…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
Changes overview
Introduced
cv::gapi::wip::ov::benchmark_mode{}compile argument which if enabled forceOpenVINObackend 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-L139Pull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.