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

initDSProfile: correct std::vector usage #4124

Merged
merged 2 commits into from Sep 13, 2023

Conversation

stima
Copy link
Contributor

@stima stima commented Sep 12, 2023

Inside initDSProfile use std::vector::resize instead of std::vector::reserve, due to later usage it by index that triggers debug cheks (reserver does not change size) under MSVC.

profile->devices.reserve(profile->numDevices);
memset(&profile->devices[0], 0, profile->numDevices * sizeof(ds_device));
profile->devices.resize(profile->numDevices);
memset(profile->devices.data(), 0, profile->numDevices * sizeof(ds_device));
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the memset still needed? The resize method should already fill all new elements with zero.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, it initializes it with default. My intent was to minimize changes and follow safe programming (e.g. who said that it wouldn't have any values inserted before). So may adress it here (in that PR) or suggest a small refactoring of that function (as for me it has a mix of c/c++ and unndeded complexity) done by me in next PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you plan to use Tesseract with OpenCL or are you already using it? In my tests (done some years ago) it did not work good enough to justify more efforts to improve that code.

Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of modifying line 194 I suggest to remove it in the PR here. Additional changes can be done in another PR later.

Copy link
Contributor Author

@stima stima Sep 13, 2023

Choose a reason for hiding this comment

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

Do you plan to use Tesseract with OpenCL or are you already using it? In my tests (done some years ago) it did not work good enough to justify more efforts to improve that code.

Doing some PoCs to see if I can use it.

Comment on lines 193 to 194
profile->devices.resize(profile->numDevices);
memset(profile->devices.data(), 0, profile->numDevices * sizeof(ds_device));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
profile->devices.resize(profile->numDevices);
memset(profile->devices.data(), 0, profile->numDevices * sizeof(ds_device));
profile->devices.resize(profile->numDevices);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@stweil stweil left a comment

Choose a reason for hiding this comment

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

Thanks!

@stweil stweil merged commit 39222a0 into tesseract-ocr:main Sep 13, 2023
@amitdo amitdo added the OpenCL label Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants