Skip to content

[Data] Add optional blocksize_elements to BlockingParameters#344

Merged
rfsaliev merged 4 commits into
mainfrom
rfsaliev/blocksize-elements
Jun 17, 2026
Merged

[Data] Add optional blocksize_elements to BlockingParameters#344
rfsaliev merged 4 commits into
mainfrom
rfsaliev/blocksize-elements

Conversation

@rfsaliev

Copy link
Copy Markdown
Member

This pull request enhances the flexibility and predictability of blocked data structures by introducing explicit control over the number of elements per block (blocksize_elements) in addition to the existing byte-based blocking (blocksize_bytes). It also improves test coverage to verify the new behavior and edge cases.

Blocking parameter improvements:

  • Added an optional blocksize_elements field to the BlockingParameters struct, allowing users to specify the number of elements per block directly. If set, this takes precedence over blocksize_bytes when determining block size. (include/svs/core/data/simple.h)

Testing and validation:

  • Added test to demonstrate the pitfalls of relying solely on blocksize_bytes for memory prediction.
  • Extended unit tests to cover scenarios where blocksize_elements is set, including checks for correct block size selection and memory consumption predictions.
    (tests/svs/core/data/block.cpp)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds an element-count-based blocking option to make blocked dataset sizing more predictable across element types/dimensions, while preserving the existing byte-based configuration.

Changes:

  • Extended svs::data::BlockingParameters with optional blocksize_elements (takes precedence when present).
  • Updated blocked SimpleData construction to compute blocksize_ from blocksize_elements when provided, otherwise fall back to blocksize_bytes.
  • Added unit tests (with a recording allocator) to demonstrate blocksize_bytes pitfalls and validate deterministic behavior with blocksize_elements.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
include/svs/core/data/simple.h Adds blocksize_elements and updates blocked SimpleData to prefer explicit element-based block sizing.
tests/svs/core/data/block.cpp Adds tests and a recording allocator to validate new block sizing behavior and memory prediction differences.

Comment thread tests/svs/core/data/block.cpp Outdated
Comment thread tests/svs/core/data/block.cpp Outdated
Comment thread include/svs/core/data/simple.h
@rfsaliev

Copy link
Copy Markdown
Member Author

/intelci

@rfsaliev rfsaliev force-pushed the rfsaliev/blocksize-elements branch from c408423 to 9522baf Compare June 17, 2026 08:03
@rfsaliev rfsaliev enabled auto-merge (squash) June 17, 2026 08:03
@rfsaliev rfsaliev merged commit bbe38c8 into main Jun 17, 2026
20 checks passed
@rfsaliev rfsaliev deleted the rfsaliev/blocksize-elements branch June 17, 2026 08:34
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