Skip to content

Extract pub const NAME for all array encoding structs#6990

Closed
jiaqizho wants to merge 1 commit intovortex-data:developfrom
jiaqizho:change-const-name-replace-id-ref
Closed

Extract pub const NAME for all array encoding structs#6990
jiaqizho wants to merge 1 commit intovortex-data:developfrom
jiaqizho:change-const-name-replace-id-ref

Conversation

@jiaqizho
Copy link

Summary

Add a pub const NAME: &str to every array encoding struct and derive pub const ID: ArrayId from Self::NAME instead of an inline string literal. This eliminates duplicated encoding name strings and gives external code a stable constant to reference when matching on encoding IDs.

relative to #6974

Testing

N/A

Add a `pub const NAME: &str` to every array encoding struct and derive `pub const ID: ArrayId` from `Self::NAME` instead of an inline string literal. This eliminates duplicated encoding name strings and gives external code a stable constant to reference when matching on encoding IDs.

Signed-off-by: jiaqizho <jiaqi.zhou@zilliz.com>
Copy link
Contributor

@gatesn gatesn left a comment

Choose a reason for hiding this comment

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

I'm not sure I understand the goal here? Why is there duplicate strings in the existing world but not in the new world?

@connortsui20
Copy link
Contributor

Was there a reason for this change in relation to #6974?

I think I still personally like this change but the reasoning isn't super clear here, where are the duplicated strings in the current codebase?

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 17, 2026

Merging this PR will improve performance by 18.36%

⚡ 2 improved benchmarks
✅ 1007 untouched benchmarks
⏩ 1515 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation bitwise_not_vortex_buffer_mut[128] 376.1 ns 317.8 ns +18.36%
Simulation bitwise_not_vortex_buffer_mut[1024] 535.6 ns 477.2 ns +12.22%

Comparing jiaqizho:change-const-name-replace-id-ref (89580d4) with develop (91e4e3f)

Open in CodSpeed

Footnotes

  1. 1515 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@jiaqizho
Copy link
Author

Was there a reason for this change in relation to #6974?

I think I still personally like this change but the reasoning isn't super clear here, where are the duplicated strings in the current codebase?

Yes, after refactoring the implementation from #6974, this change is no longer needed, so closed.

@jiaqizho jiaqizho closed this Mar 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants