Skip to content
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

Fix StressMemLeaksTests with several models #19986

Merged
merged 2 commits into from
Sep 21, 2023

Conversation

vurusovs
Copy link
Contributor

Details:

  • Because ie_wrapper was captured by reference, tests actually execute only last model instead of several models

@vurusovs vurusovs requested a review from a team as a code owner September 21, 2023 08:40
@vurusovs vurusovs added this to the 2023.2 milestone Sep 21, 2023
@@ -97,14 +95,14 @@ infer_request_inference(const std::string &model, const std::string &target_devi


std::function<void()> reinfer_request_inference(std::shared_ptr<InferApiBase> &ie_wrapper) {
return [&] {
return [=] {
ie_wrapper->infer();
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure if it's right here, from my understand of the test code, maybe I am wrong, even if use [=] here, when test_params.numthreads > 1, there will be multi threads use same ie_wrapper to call infer(); one of them will do infer and other threads will throw request busy. so please help to set test_params.numthreads=10 and check if the test case is ok, thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for detailed review! The original issue was in consecutive inference, each inference request in separate thread (code snippet may be found in CVS-111490). So it shouldn't cause issues with ie_wrapper

test_params.numthreads is a legacy from StressUnitTests, StressMemLeakTests weren't designed to test parallel execution because it is non-trivial

@p-durandin p-durandin enabled auto-merge (squash) September 21, 2023 13:24
@p-durandin p-durandin merged commit b00fbd0 into openvinotoolkit:master Sep 21, 2023
alvoron pushed a commit to alvoron/openvino that referenced this pull request Nov 6, 2023
* Fix `StressMemLeaksTests` with several models

* Fix OMZ branch name in `get_testdata.py`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants