Skip to content

Add avx512vl and avx512vpopcntdq in telemetry#7471

Merged
JojiiOfficial merged 1 commit intodevfrom
avx512_in_telemetry
Oct 29, 2025
Merged

Add avx512vl and avx512vpopcntdq in telemetry#7471
JojiiOfficial merged 1 commit intodevfrom
avx512_in_telemetry

Conversation

@JojiiOfficial
Copy link
Contributor

@JojiiOfficial JojiiOfficial commented Oct 29, 2025

Depends on #7052

In #7052 we use new x86 features because of avx512 usage. This PR adds them into telemetry for consistency and debugging purposes.

@IvanPleshkov IvanPleshkov force-pushed the avx512-for-binary-quantization branch from 44e87a8 to 9ef1ca7 Compare October 29, 2025 11:54
Base automatically changed from avx512-for-binary-quantization to dev October 29, 2025 12:26
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 29, 2025

📝 Walkthrough

Walkthrough

This change extends CPU feature detection in the telemetry module to recognize two additional x86 instruction set extensions: avx512vl and avx512vpopcntdq. The detection logic adds these flags to the cpu_flags vector that gets captured in RunningEnvironmentTelemetry. This is a straightforward addition to the existing feature detection pattern with no changes to surrounding logic or function signatures.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Single file modification with repetitive pattern additions
  • No logic changes or structural modifications
  • Straightforward extension of existing CPU flag detection

Possibly related PRs

  • avx512 for binary quantization #7052: Implements runtime detection and usage of avx512vl and avx512vpopcntdq feature flags in an AVX512 codepath, directly complementing the telemetry reporting added in this change.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title "Add avx512vl and avx512vpopcntdq in telemetry" directly and accurately describes the main change in the pull request. The changeset extends x86 feature detection in the telemetry system by adding two new CPU flags (avx512vl and avx512vpopcntdq) to the cpu_flags collection. The title is concise, specific, and clearly communicates the primary purpose of the change without being overly broad or vague.
Description Check ✅ Passed The pull request description is directly related to the changeset. It provides context about the dependency on PR #7052, explains that new x86 features were introduced for AVX-512 usage, and clarifies the purpose of adding these features to telemetry for consistency and debugging. The description is meaningful and not vague or generic, as it specifically references the x86 features and their motivation.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch avx512_in_telemetry
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f107ee9 and af2697a.

📒 Files selected for processing (1)
  • src/common/telemetry_ops/app_telemetry.rs (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs

📄 CodeRabbit inference engine (.github/review-rules.md)

**/*.rs: Prefer explicit SomeType::from(x) over implicit x.into() in Rust code
Do not use transmute_from_u8, transmute_to_u8, transmute_from_u8_to_slice, transmute_from_u8_to_mut_slice, transmute_to_u8_slice in new code; use bytemuck or zerocopy instead

Files:

  • src/common/telemetry_ops/app_telemetry.rs
**/src/**/*.rs

📄 CodeRabbit inference engine (.github/review-rules.md)

**/src/**/*.rs: Prefer exhaustive match arms over a catch-all _ arm to avoid missing new enum variants (except in tests/benchmarks or when provably safe)
Prefer explicit field ignoring with : _ over .. in struct patterns (except in tests/benchmarks or when provably safe)

Files:

  • src/common/telemetry_ops/app_telemetry.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: integration-tests-consensus
  • GitHub Check: test-consistency
  • GitHub Check: test-shard-snapshot-api-s3-minio
  • GitHub Check: e2e-tests
  • GitHub Check: integration-tests
  • GitHub Check: test-consensus-compose
  • GitHub Check: rust-tests (windows-latest)
  • GitHub Check: rust-tests-no-rocksdb (ubuntu-latest)
  • GitHub Check: rust-tests (macos-latest)
  • GitHub Check: rust-tests (ubuntu-latest)
  • GitHub Check: lint
🔇 Additional comments (1)
src/common/telemetry_ops/app_telemetry.rs (1)

138-143: LGTM! AVX-512 feature detection correctly implemented.

The additions of avx512vl and avx512vpopcntdq follow the established pattern and are correctly placed within the x86/x86_64 feature detection block. Both are valid AVX-512 instruction set extensions that will be useful for telemetry and debugging purposes.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot mentioned this pull request Oct 29, 2025
Open
@JojiiOfficial JojiiOfficial merged commit b3c6f40 into dev Oct 29, 2025
15 checks passed
@JojiiOfficial JojiiOfficial deleted the avx512_in_telemetry branch October 29, 2025 13:00
timvisee pushed a commit that referenced this pull request Nov 14, 2025
@JojiiOfficial @timvisee
Add avx512vl and avx512vpopcntdq in telemetry (#7471)
30fc847
@timvisee timvisee mentioned this pull request Nov 14, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@IvanPleshkov IvanPleshkov IvanPleshkov approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@JojiiOfficial @IvanPleshkov