Skip to content

chore: refactor storable/tuples for better readability#323

Merged
maksymar merged 17 commits intomainfrom
maksym/tuple-cleanup
May 28, 2025
Merged

chore: refactor storable/tuples for better readability#323
maksymar merged 17 commits intomainfrom
maksym/tuple-cleanup

Conversation

@maksymar
Copy link
Copy Markdown
Contributor

@maksymar maksymar commented May 26, 2025 •
edited
Loading

This PR refactors the tuples file for improved readability without altering functionality. Key changes include:

Renaming variables (e.g., replacing a_max_size with a_max) for clarity.
Consolidating and streamlining offset handling in serialization/deserialization.
Converting some vector parameters to slices and inlining match definitions.

@maksymar maksymar requested a review from Copilot May 26, 2025 14:22
@maksymar maksymar changed the title chore: refactoring tuples chore: refactor storable/tuples for better readability May 26, 2025

This comment was marked as outdated.

Sign in to view

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 26, 2025 •
edited
Loading

canbench 🏋 (dir: ./benchmarks/memory_manager) b31c9f0 2025-05-28 05:35:52 UTC

✅ ./benchmarks/memory_manager/canbench_results.yml is up to date
📦 canbench_results_memory-manager.csv available in artifacts

---------------------------------------------------

Summary:
  instructions:
    status:   No significant changes detected 👍
    counts:   [total 3 | new 0 | improved 0 | regressed 0 | unchanged 3]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  heap_increase:
    status:   No significant changes detected 👍
    counts:   [total 3 | new 0 | improved 0 | regressed 0 | unchanged 3]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  stable_memory_increase:
    status:   No significant changes detected 👍
    counts:   [total 3 | new 0 | improved 0 | regressed 0 | unchanged 3]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

---------------------------------------------------
CSV results saved to canbench_results.csv

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 26, 2025 •
edited
Loading

canbench 🏋 (dir: ./benchmarks/vec) b31c9f0 2025-05-28 05:36:02 UTC

✅ ./benchmarks/vec/canbench_results.yml is up to date
📦 canbench_results_vec.csv available in artifacts

---------------------------------------------------

Summary:
  instructions:
    status:   No significant changes detected 👍
    counts:   [total 16 | new 0 | improved 0 | regressed 0 | unchanged 16]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  heap_increase:
    status:   No significant changes detected 👍
    counts:   [total 16 | new 0 | improved 0 | regressed 0 | unchanged 16]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  stable_memory_increase:
    status:   No significant changes detected 👍
    counts:   [total 16 | new 0 | improved 0 | regressed 0 | unchanged 16]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

---------------------------------------------------
CSV results saved to canbench_results.csv

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 26, 2025 •
edited
Loading

canbench 🏋 (dir: ./benchmarks/btreemap) b31c9f0 2025-05-28 05:37:43 UTC

✅ ./benchmarks/btreemap/canbench_results.yml is up to date
📦 canbench_results_btreemap.csv available in artifacts

---------------------------------------------------

Summary:
  instructions:
    status:   No significant changes detected 👍
    counts:   [total 285 | new 0 | improved 0 | regressed 0 | unchanged 285]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  heap_increase:
    status:   No significant changes detected 👍
    counts:   [total 285 | new 0 | improved 0 | regressed 0 | unchanged 285]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  stable_memory_increase:
    status:   No significant changes detected 👍
    counts:   [total 285 | new 0 | improved 0 | regressed 0 | unchanged 285]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

---------------------------------------------------
CSV results saved to canbench_results.csv

@maksymar maksymar requested a review from Copilot May 26, 2025 14:29

This comment was marked as outdated.

Sign in to view

@maksymar maksymar requested a review from Copilot May 26, 2025 14:35
Copilot AI reviewed May 26, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the tuples file for improved readability without altering functionality. Key changes include:

  • Renaming variables (e.g., replacing a_max_size with a_max) for clarity.
  • Consolidating and streamlining offset handling in serialization/deserialization.
  • Converting some vector parameters to slices and inlining match definitions.

@maksymar maksymar marked this pull request as ready for review May 26, 2025 14:39
@maksymar maksymar requested a review from a team as a code owner May 26, 2025 14:39
berestovskyy
berestovskyy reviewed May 26, 2025
View reviewed changes
Comment thread src/storable/tuples.rs Outdated
berestovskyy
berestovskyy reviewed May 26, 2025
View reviewed changes
Comment thread src/storable/tuples.rs Outdated
berestovskyy
berestovskyy reviewed May 26, 2025
View reviewed changes
Comment thread src/storable/tuples.rs Outdated
berestovskyy
berestovskyy reviewed May 26, 2025
View reviewed changes
Comment thread src/storable/tuples.rs Outdated
berestovskyy
berestovskyy reviewed May 26, 2025
View reviewed changes
Comment thread src/storable/tuples.rs Outdated
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 27, 2025 •
edited
Loading

canbench 🏋 (dir: ./benchmarks/compare) b31c9f0 2025-05-28 05:36:55 UTC

✅ ./benchmarks/compare/canbench_results.yml is up to date
📦 canbench_results_compare.csv available in artifacts

---------------------------------------------------

Summary:
  instructions:
    status:   Regressions detected! 🔴
    counts:   [total 18 | new 0 | improved 0 | regressed 2 | unchanged 16]
    change:   [max +164.24M | p75 +56 | median +21 | p25 0 | min -172]
    change %: [max +4.59% | p75 0.00% | median 0.00% | p25 0.00% | min -0.00%]

  heap_increase:
    status:   Regressions detected! 🔴
    counts:   [total 18 | new 0 | improved 0 | regressed 4 | unchanged 14]
    change:   [max +1.89K | p75 0 | median 0 | p25 0 | min 0]
    change %: [max +100.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  stable_memory_increase:
    status:   No significant changes detected 👍
    counts:   [total 18 | new 0 | improved 0 | regressed 0 | unchanged 18]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

---------------------------------------------------

Only significant changes:
| status | name                | calls |   ins |  ins Δ% |    HI |    HI Δ% |   SMI |  SMI Δ% |
|--------|---------------------|-------|-------|---------|-------|----------|-------|---------|
|   +    | write_chunks_vec_1m |       | 3.74B |  +4.59% | 3.78K | +100.00% | 1.67K |   0.00% |
|   +    | read_chunks_vec_1m  |       | 4.75B |  +3.58% | 3.78K | +100.00% | 1.67K |   0.00% |
|   +    | write_chunks_vec_1k |       | 1.27B |  +0.04% | 3.20K |  +99.75% | 1.67K |   0.00% |
|   +    | read_chunks_vec_1k  |       | 1.38B |  +0.03% | 3.20K |  +99.75% | 1.67K |   0.00% |

ins = instructions, HI = heap_increase, SMI = stable_memory_increase, Δ% = percent change

---------------------------------------------------
CSV results saved to canbench_results.csv

@maksymar
Copy link
Copy Markdown
Contributor Author

maksymar commented May 28, 2025 •
edited
Loading

This change has been confirmed on a fresh main branch to be related to a third-party dependency update.

canbench 🏋 (dir: ./benchmarks/compare) 44090bf 2025-05-28 05:44:42 UTC

✅ ./benchmarks/compare/canbench_results.yml is up to date
📦 canbench_results_compare.csv available in artifacts

---------------------------------------------------

Summary:
  instructions:
    status:   Regressions detected! 🔴
    counts:   [total 18 | new 0 | improved 0 | regressed 2 | unchanged 16]
    change:   [max +164.24M | p75 +56 | median +21 | p25 0 | min -172]
    change %: [max +4.59% | p75 0.00% | median 0.00% | p25 0.00% | min -0.00%]

  heap_increase:
    status:   Regressions detected! 🔴
    counts:   [total 18 | new 0 | improved 0 | regressed 4 | unchanged 14]
    change:   [max +1.89K | p75 0 | median 0 | p25 0 | min 0]
    change %: [max +100.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  stable_memory_increase:
    status:   No significant changes detected 👍
    counts:   [total 18 | new 0 | improved 0 | regressed 0 | unchanged 18]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

---------------------------------------------------

Only significant changes:
| status | name                | calls |   ins |  ins Δ% |    HI |    HI Δ% |   SMI |  SMI Δ% |
|--------|---------------------|-------|-------|---------|-------|----------|-------|---------|
|   +    | write_chunks_vec_1m |       | 3.74B |  +4.59% | 3.78K | +100.00% | 1.67K |   0.00% |
|   +    | read_chunks_vec_1m  |       | 4.75B |  +3.58% | 3.78K | +100.00% | 1.67K |   0.00% |
|   +    | write_chunks_vec_1k |       | 1.27B |  +0.04% | 3.20K |  +99.75% | 1.67K |   0.00% |
|   +    | read_chunks_vec_1k  |       | 1.38B |  +0.03% | 3.20K |  +99.75% | 1.67K |   0.00% |

ins = instructions, HI = heap_increase, SMI = stable_memory_increase, Δ% = percent change

---------------------------------------------------
CSV results saved to canbench_results.csv

@maksymar maksymar enabled auto-merge (squash) May 28, 2025 05:51
@maksymar maksymar merged commit 185d58e into main May 28, 2025
14 checks passed
@maksymar maksymar deleted the maksym/tuple-cleanup branch May 28, 2025 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

+1 more reviewer

@berestovskyy berestovskyy berestovskyy approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@maksymar @berestovskyy