Skip to content

refactor(data): replace PartialVec tree structure with flat entry list#747

Merged
vapourismo merged 1 commit intomainfrom
ole/push-qnxkroukwssp
Feb 2, 2026
Merged

refactor(data): replace PartialVec tree structure with flat entry list#747
vapourismo merged 1 commit intomainfrom
ole/push-qnxkroukwssp

Conversation

@vapourismo
Copy link
Copy Markdown
Collaborator

@vapourismo vapourismo commented Jan 24, 2026

Relates to RV-861

What

Simplifies the PartialVec data structure. It maintains its memory-efficient representation but uses a flat list of entries instead of a tree structure.

While this makes the code simpler and safer, it comes at the cost of worse time complexity for insertions, as range entries need to be moved in memory to maintain order. The data itself doesn't require moving, but the anchoring entries do.

In essence, this PR trades efficiency for increased safety and simplicity.

Why

The tree-based approach has a critical flaw: its Drop implementation can overflow the stack when the tree is nested too deeply. Fixing this requires even more complexity to be baked into the module. To avoid more complexity, I chose to overhaul the implementation.

This PR addresses this while also making the module more accessible.

Manually Testing

make all

Tasks for the Author

  • Link all Linear issues related to this MR using magic words (e.g. part of, relates to, closes).
  • Eliminate dead code and other spurious artefacts introduced in your changes.
  • Document new public functions, methods and types.
  • Make sure the documentation for updated functions, methods, and types is correct.
  • Add tests for bugs that have been fixed.
  • Explain changes to regression test captures when applicable.
  • Write commit messages in agreement with our guidelines.
  • Self-review your changes to ensure they are high-quality.
  • Complete all of the above before assigning this MR to reviewers.

@vapourismo vapourismo changed the base branch from main to ole/push-xlxtlttxyrws January 24, 2026 00:19
@vapourismo vapourismo force-pushed the ole/push-qnxkroukwssp branch from 747e440 to 54edab0 Compare January 24, 2026 00:21
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 24, 2026

Codecov Report

❌ Patch coverage is 98.37398% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.97%. Comparing base (1dbfb94) to head (297c5e4).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
data/src/components/bytes.rs 0.00% 1 Missing ⚠️
data/src/partial_vec.rs 99.15% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #747      +/-   ##
==========================================
- Coverage   89.99%   89.97%   -0.02%     
==========================================
  Files         109      109              
  Lines       20672    20656      -16     
  Branches    20672    20656      -16     
==========================================
- Hits        18603    18585      -18     
- Misses       1690     1692       +2     
  Partials      379      379              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vapourismo vapourismo force-pushed the ole/push-qnxkroukwssp branch from 54edab0 to 6327543 Compare January 24, 2026 00:42
@vapourismo vapourismo force-pushed the ole/push-qnxkroukwssp branch from 6327543 to 50cc766 Compare January 24, 2026 00:52
@vapourismo vapourismo force-pushed the ole/push-xlxtlttxyrws branch from 194c58a to 59ef99a Compare January 24, 2026 01:02
@vapourismo vapourismo force-pushed the ole/push-qnxkroukwssp branch 7 times, most recently from 6daf57b to 397d10c Compare January 26, 2026 11:38
@vapourismo vapourismo force-pushed the ole/push-xlxtlttxyrws branch from 59ef99a to 7eb2c38 Compare January 26, 2026 12:10
@vapourismo vapourismo force-pushed the ole/push-qnxkroukwssp branch from 397d10c to 681eec3 Compare January 26, 2026 12:10
@vapourismo vapourismo force-pushed the ole/push-xlxtlttxyrws branch 2 times, most recently from 2660fee to ab3616b Compare January 26, 2026 15:43
@vapourismo vapourismo force-pushed the ole/push-qnxkroukwssp branch from 681eec3 to 71e0ee3 Compare January 26, 2026 15:45
@vapourismo vapourismo force-pushed the ole/push-xlxtlttxyrws branch from ab3616b to 6e252c0 Compare January 26, 2026 15:45
Base automatically changed from ole/push-xlxtlttxyrws to main January 26, 2026 16:25
@vapourismo vapourismo force-pushed the ole/push-qnxkroukwssp branch 5 times, most recently from a837c5e to 8099b8d Compare January 26, 2026 18:07
@vapourismo vapourismo marked this pull request as ready for review January 26, 2026 18:07
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jan 26, 2026

Benchmark results for revision fd6d89a:

Metric Duration TPS
Mean 1.520356197s 26.310
Worst 1.531294822s 26.122
Best 1.514365279s 26.414
Standard Deviation ±4.103194ms ±0.071
Full results
Run Transfers Duration TPS
1 40 1.515877803s 26.387
2 40 1.521655065s 26.287
3 40 1.521771734s 26.285
4 40 1.531294822s 26.122
5 40 1.518529289s 26.341
6 40 1.52344101s 26.256
7 40 1.519766437s 26.320
8 40 1.522522009s 26.272
9 40 1.525675027s 26.218
10 40 1.518845156s 26.336
11 40 1.514540428s 26.411
12 40 1.514588238s 26.410
13 40 1.51801146s 26.350
14 40 1.518383174s 26.344
15 40 1.521744559s 26.286
16 40 1.518699485s 26.338
17 40 1.524608567s 26.236
18 40 1.519460434s 26.325
19 40 1.514365279s 26.414
20 40 1.523343968s 26.258

Compare the results above with those for the default branch.

@vapourismo vapourismo force-pushed the ole/push-qnxkroukwssp branch 4 times, most recently from e1972da to 61f279d Compare January 28, 2026 11:48
Comment thread data/src/partial_vec.rs
Comment thread data/src/partial_vec.rs
Comment thread data/src/partial_vec.rs Outdated
Comment thread data/src/partial_vec.rs Outdated
Comment thread data/src/partial_vec.rs Outdated
@vapourismo vapourismo force-pushed the ole/push-qnxkroukwssp branch 5 times, most recently from 0db28c3 to a939292 Compare January 30, 2026 12:21
@vapourismo vapourismo requested a review from emturner January 30, 2026 12:21
@vapourismo vapourismo force-pushed the ole/push-qnxkroukwssp branch from a939292 to 98abd54 Compare January 30, 2026 14:47
Copy link
Copy Markdown
Contributor

@NSant215 NSant215 left a comment

Choose a reason for hiding this comment

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

there is a PartialVec::undefined that has been missed in bytes.rs that is causing local build to fail

@vapourismo vapourismo force-pushed the ole/push-qnxkroukwssp branch from 98abd54 to 4747120 Compare January 30, 2026 16:32
The previous binary tree implementation (Undefined/Defined/Concatenated
enum) suffered from unbounded tree depth when many small ranges were
defined, risking stack overflow during traversal.

Replace with a flat Vec<DefinedEntry> sorted by start index, where gaps
implicitly represent undefined regions. This eliminates recursion depth
issues and simplifies the implementation:

- `define()` uses binary search to find insertion points
- `truncate()` and `range()` become simple linear operations
- `undefined()` no longer takes a length parameter (gaps are implicit)
- Removes unsafe pointer manipulation from truncate()

The new structure maintains O(log n) lookup via partition_point while
avoiding the fragmentation and depth issues of the tree approach.
@vapourismo vapourismo force-pushed the ole/push-qnxkroukwssp branch from 4747120 to 297c5e4 Compare January 30, 2026 17:01
@vapourismo
Copy link
Copy Markdown
Collaborator Author

there is a PartialVec::undefined that has been missed in bytes.rs that is causing local build to fail

Thanks! Fixed now.

Copy link
Copy Markdown
Contributor

@NSant215 NSant215 left a comment

Choose a reason for hiding this comment

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

LGTM

@vapourismo vapourismo added this pull request to the merge queue Feb 2, 2026
Merged via the queue into main with commit 4606c08 Feb 2, 2026
8 checks passed
@vapourismo vapourismo deleted the ole/push-qnxkroukwssp branch February 2, 2026 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants