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

perf(chore): optimize binary search corner cases #4402

Merged

Conversation

sivukhin
Copy link
Contributor

@sivukhin sivukhin commented Apr 14, 2024

Context

Several implementations of binary search in QuestDB have same bug in it - when reaching small range length binary search fallback to linear search, but without any lower bound on index. So, basically algorithms works in linear time (not O(log n)).

This PR fixes a bunch of such similar bugs and start linear scan from lower bound calculated during binary search routine.

Initially this PR were written in kind of "humorous" style. But, as it can violate code of conduct and actually confuses more then helps - I rewrote PR description but leave original text for historical reference

The labor union of algorithms reported that QuestDB (QDB) violates the rights of "binary search algorithm" (BSA) by requiring it to work without utilizing any of its results. The BSA has earned trust through its extensive use in production applications, making the complaint valid. Therefore, QDB must respect the rights of the algorithm.

During audit 4 violations were found.

One concrete example is here (commit hash: a077bee, file: IntLongPriorityQueue.java, line: 109, link)

private int binSearch0(long v) {
    int low = 0;
    int high = size;

    while (low < high) {

        if (high - low < 65) {
            return scanSearch(v); // binary search work completely ignored, so rude!
        }

        int mid = (low + high - 1) >>> 1;
        long midVal = buf.getQuick(mid);

        if (midVal < v)
            low = mid + 1;
        else if (midVal > v)
            high = mid;
        else {
            while (++mid < high && buf.getQuick(mid) == v) {
            }
            return mid;
        }
    }
    return low;
}

QDB must respect rights of algorithms, especially such honorable as BSA. PR fixes this uncomfortable situation.

Hope that all legal rights of algorithms will be respected in such amazing project as QDB from now on.

@bluestreak01
Copy link
Member

bluestreak01 commented Apr 14, 2024

Hi Nikita (@sivukhin), could you help me understand this report:

The labor union of algorithms reported that QuestDB (QDB) violates the rights of "binary search algorithm" (BSA)

by providing link with more information, perhaps the reference on the aforementioned report.

Or is this a joke?

@sivukhin
Copy link
Contributor Author

sivukhin commented Apr 14, 2024

Or is this a joke?

Yes, sorry, I guess bad one. For some reason this mistake made me laugh a bit.
I can for sure rewrite PR description if it's required.

@puzpuzpuz puzpuzpuz changed the title perf: regain trust to the binary search perf(chore): optimize binary search corner cases Apr 15, 2024
@puzpuzpuz puzpuzpuz added Performance Performance improvements Core Related to storage, data type, etc. labels Apr 15, 2024
@puzpuzpuz puzpuzpuz self-requested a review April 15, 2024 08:00
@puzpuzpuz
Copy link
Contributor

@sivukhin thanks for defending the honour of the noble BSA. 🛡️

@bluestreak01 bluestreak01 merged commit b22eccc into questdb:master Apr 15, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Related to storage, data type, etc. Performance Performance improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants