Skip to content

fix(vectordb): fix croaring avx#771

Merged
qin-ctx merged 1 commit intomainfrom
fix_avx
Mar 19, 2026
Merged

fix(vectordb): fix croaring avx#771
qin-ctx merged 1 commit intomainfrom
fix_avx

Conversation

@zhoujh01
Copy link
Collaborator

Description

Related Issue

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Screenshots (if applicable)

Additional Notes

@github-actions
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Over-Disable

The SSE3 variant adds CROARING_DISABLE_X64=1. Verify if disabling x64 for croaring in the SSE3 case is intended and does not break functionality on x86-64 platforms.

list(APPEND OV_DEFS CROARING_DISABLE_X64=1)

@qin-ctx qin-ctx merged commit 4984ba8 into main Mar 19, 2026
30 checks passed
@qin-ctx qin-ctx deleted the fix_avx branch March 19, 2026 09:29
@github-project-automation github-project-automation bot moved this from Backlog to Done in OpenViking project Mar 19, 2026
@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Remove unnecessary x64 disable define

Remove the CROARING_DISABLE_X64=1 define from the SSE3 variant configuration. This
define is not present in the AVX2 section, and may unnecessarily disable 64-bit
optimizations in croaring for x86-64 builds, potentially leading to suboptimal
performance or build issues.

src/CMakeLists.txt [146]

-list(APPEND OV_DEFS CROARING_DISABLE_X64=1)
 
+
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that CROARING_DISABLE_X64=1 is only present in the SSE3 section (not AVX2) and could unnecessarily disable 64-bit optimizations in croaring, which may impact performance.

Medium

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants