Skip to content
This repository was archived by the owner on Jan 26, 2026. It is now read-only.

#14 - Implement 2-Byte Field IDs and Update Directory Entry Structure#35

Merged
agavra merged 3 commits intoimprint-serde:mainfrom
hemantkrsh:feature/issue-14-2byte-fields
Jun 7, 2025
Merged

#14 - Implement 2-Byte Field IDs and Update Directory Entry Structure#35
agavra merged 3 commits intoimprint-serde:mainfrom
hemantkrsh:feature/issue-14-2byte-fields

Conversation

@hemantkrsh
Copy link
Contributor

Implement 2-Byte Field IDs and Update Directory Entry Structure

This PR addresses issue #14 by implementing support for 2-byte Field IDs and consequently updating the DirectoryEntry structure. These changes provide greater flexibility for defining fields and optimize the storage of directory entries.


Key Changes:

  • types.rs:

    • The DirectoryEntry struct has been modified to accommodate the new 2-byte Field IDs.
    • Necessary changes have been made to the get_value and get_raw_bytes functions within ImprintRecord impl to correctly handle the Field ID size.
  • ops.rs:

    • The project function signature within the Project trait has been updated to reflect changes related to Field ID size.
    • The implementation of the Project trait has been modified accordingly to handle the new Field ID size.
    • The test should_project_subset_of_fields has been updated to reflect the new Field ID size.
  • serde.rs:

    • The constant DIR_ENTRY_BYTES has been updated to reflect the new size of a DirectoryEntry (now 7 bytes, including the 2-byte Field ID).
    • The Read and Write implementations for DirectoryEntry have been modified to correctly serialize and deserialize the new 7-byte format, including the 2-byte Field ID.
  • writer.rs:

    • The key type of the BTreeMap within the ImprintWriter struct and its implementation has been changed to u16..
  • README.md and FORMAT.md:

    • The documentation in both README.md and FORMAT.md has been updated to reflect that the DirectoryEntry size is now 7 bytes.

These changes ensure that the data structures and serialization/deserialization logic correctly handle 2-byte Field IDs and the revised DirectoryEntry format, while also updating the documentation to reflect the change.

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. documentation Improvements or additions to documentation enhancement New feature or request labels Jun 7, 2025
Copy link
Contributor

@agavra agavra 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 @hemantkrsh - this looks good to me.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 7, 2025
@agavra
Copy link
Contributor

agavra commented Jun 7, 2025

Benchmark failed for permissions to post on the PR:

Here is your content converted into true Markdown, with proper escaping removed and formatting corrected:

📊 Benchmark Comparison Report

This pull request includes Criterion benchmarks comparing performance to the main branch.
This comment will automatically update as the benchmarks are re-run on each commit.

The table below shows relative ratios and timing stats for each benchmark group:

group                main                                   pr

---

ops/merge            1.00  1389.3±16.25ns        ? ?/sec    1.04  1445.7±16.71ns        ? ?/sec
ops/project          1.00    521.7±9.46ns        ? ?/sec    1.16    605.2±3.23ns        ? ?/sec
serde/deserialize    1.00    274.8±7.56ns        ? ?/sec    1.02    279.5±2.31ns        ? ?/sec
serde/serialize      1.00    261.7±6.26ns        ? ?/sec    1.02    267.8±5.78ns        ? ?/sec

✅ Benchmarks completed successfully.

🧠 Notes:

  • These benchmarks are not a pass/fail gate and are informative only.
  • Use this as a signal to review performance-sensitive changes.
  • Results may be unreliable due to GHA runner hardware variance.
  • If results indicate a significant performance regression, run the benchmarks locally to confirm.

Reported by the benchmark CI bot

Let me know if you'd like it formatted for a specific platform like GitHub or for inline use elsewhere.

@agavra agavra merged commit 97eb5c8 into imprint-serde:main Jun 7, 2025
1 of 2 checks passed
@agavra agavra mentioned this pull request Jun 7, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

documentation Improvements or additions to documentation enhancement New feature or request lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants