Add debug assert to check in FLANN the vectors size is multiple of th…#18085
Merged
opencv-pushbot merged 1 commit intoopencv:3.4from Aug 20, 2020
Merged
Conversation
…e architecture word size
Contributor
|
@vpisarev Please take a look. |
alalek
reviewed
Aug 15, 2020
| template <typename Iterator1, typename Iterator2> | ||
| ResultType operator()(const Iterator1 a, const Iterator2 b, size_t size, ResultType /*worst_dist*/ = -1) const | ||
| { | ||
| CV_DbgAssert(!(size % long_word_size_) && "vectors size must be multiple of long words size (i.e. 8)"); |
Member
There was a problem hiding this comment.
Is it frequently used function (for example called from some loop)? Is overhead of this check measurable?
If not then my suggestion is to use CV_Assert() instead to keep this check in Release builds too (almost all binary packages + python bindings).
Or we need to add an upper level validation check for that (which is called once to reduce overhead).
Contributor
Author
|
This is the core of the inner loops of the inner loops. for binary
descriptors (CENSUS, BRIEF, BRISK, FREAK, KAZE, AKAZE) it is the method
that computes the Hamming distance between two binary vectors. This is the
base for parsing trees in FLANN.
If you take a standard vector of 512 bits, this means the loop doing a
binary operation between both vectors (1 cycle on most HW) will iterate 8
times over the 64 Bytes of the vectors. Adding simply a modulo computation
here would most likely arm significantly the processing time.
It was the fastest way to add this test, but yes, I could check for placing
tests at a higher level .
Le dim. 16 août 2020 à 00:34, Alexander Alekhin <notifications@github.com>
a écrit :
… ***@***.**** commented on this pull request.
Thank you for contribution!
------------------------------
In modules/flann/include/opencv2/flann/dist.h
<#18085 (comment)>:
> @@ -683,6 +683,8 @@ struct Hamming2
template <typename Iterator1, typename Iterator2>
ResultType operator()(const Iterator1 a, const Iterator2 b, size_t size, ResultType /*worst_dist*/ = -1) const
{
+ CV_DbgAssert(!(size % long_word_size_) && "vectors size must be multiple of long words size (i.e. 8)");
Is it frequently used function (for example called from some loop)? Is
overhead of this check measurable?
If not then my suggestion is to use CV_Assert() instead to keep this
check in Release builds too (almost all binary packages + python bindings).
Or we need to add an upper level validation check for that (which is
called once to reduce overhead).
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#18085 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABPELVM2JH2JJHTJVML7DNLSA4EP7ANCNFSM4P5JRRKA>
.
|
alalek
approved these changes
Aug 20, 2020
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…e architecture word size
Pull 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.