Skip to content

feat: introduce common/memory module#11

Merged
JingsongLi merged 2 commits into
apache:mainfrom
lxy-9602:add-common-data-types
May 26, 2026
Merged

feat: introduce common/memory module#11
JingsongLi merged 2 commits into
apache:mainfrom
lxy-9602:add-common-data-types

Conversation

@lxy-9602
Copy link
Copy Markdown
Contributor

Purpose

No Linked issue.

Introduce

  • MemorySegment — core memory abstraction wrapping a Bytes buffer with typed get/put operations, bounds checking, and bulk copy/compare support (memory_segment.h/cpp)
  • MemorySegmentUtils — utility functions for cross-segment operations including multi-segment copy, compare, equality check, and hash (memory_segment_utils.h/cpp)

Tests

  • memory_segment_test.cpp — MemorySegment typed read/write, boundary checks, bulk operations
  • memory_segment_utils_test.cpp — Multi-segment copy, compare, hash correctness

API and Format

Documentation

Generative AI tooling

Copy link
Copy Markdown

@leaves12138 leaves12138 left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution. I found one correctness issue that should be fixed before merge.

MemorySegment::Compare compares each 8-byte chunk as a native-endian integer. On little-endian platforms this does not preserve lexicographical byte order. For example, [0x01, 0, 0, 0, 0, 0, 0, 0] should compare greater than [0x00, 0x01, 0, 0, 0, 0, 0, 0] byte-wise, but the current implementation reads them as 1 and 256 and returns the opposite result.

Please either compare byte-by-byte, or use an endian-safe/big-endian 8-byte comparison, and add a regression test covering this case.

Copy link
Copy Markdown

@leaves12138 leaves12138 left a comment

Choose a reason for hiding this comment

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

Re-reviewed the latest update. The previous MemorySegment::Compare correctness blocker is fixed: the 8-byte fast path now compares big-endian values, so lexicographical byte order is preserved on little-endian platforms. I do not see further blockers.

@JingsongLi JingsongLi merged commit 534b67d into apache:main May 26, 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