Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Layout.fixed: bool for a fast path in eq_row_in_page & row_type_visitor #2025

Merged
merged 2 commits into from
Dec 3, 2024

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Nov 28, 2024

Description of Changes

Adds to Layout whether it is the layout of a "fixed type" and not the layout of a var-len type's fixed component.
This is then exploited to 2x-10x speed up eq_row_in_page and speed up row_type_visitor for the fixed len case at the cost of some minor regression for small VLOs.

This in turn will speed up insertions and subscriptions.

Benchmarking eq_in_page/U32: Collecting 100 samples in estimated 5.0000 s (1.5B iter
eq_in_page/U32          time:   [3.4175 ns 3.4689 ns 3.5373 ns]
                        change: [-55.828% -54.348% -52.236%] (p = 0.00 < 0.05)
                        Performance has improved.
Benchmarking eq_in_page/Option<U32>/None: Collecting 100 samples in estimated 5.0000
eq_in_page/Option<U32>/None
                        time:   [3.3766 ns 3.3865 ns 3.4052 ns]
                        change: [-65.726% -65.473% -65.207%] (p = 0.00 < 0.05)
                        Performance has improved.
Benchmarking eq_in_page/Option<U32>/Some: Collecting 100 samples in estimated 5.0000
eq_in_page/Option<U32>/Some
                        time:   [3.3739 ns 3.3803 ns 3.3942 ns]
                        change: [-74.766% -74.605% -74.449%] (p = 0.00 < 0.05)
                        Performance has improved.
Benchmarking eq_in_page/U32x2: Collecting 100 samples in estimated 5.0000 s (1.5B it
eq_in_page/U32x2        time:   [3.3937 ns 3.4211 ns 3.4583 ns]
                        change: [-72.515% -71.694% -71.042%] (p = 0.00 < 0.05)
                        Performance has improved.
Benchmarking eq_in_page/U32x4: Collecting 100 samples in estimated 5.0000 s (1.5B it
eq_in_page/U32x4        time:   [3.3896 ns 3.4206 ns 3.4578 ns]
                        change: [-84.075% -83.915% -83.753%] (p = 0.00 < 0.05)
                        Performance has improved.
Benchmarking eq_in_page/U32x8: Collecting 100 samples in estimated 5.0000 s (1.4B it
eq_in_page/U32x8        time:   [3.6620 ns 3.7123 ns 3.7736 ns]
                        change: [-90.794% -90.690% -90.574%] (p = 0.00 < 0.05)
                        Performance has improved.
Benchmarking eq_in_page/String/0: Collecting 100 samples in estimated 5.0000 s (673M
eq_in_page/String/0     time:   [7.5180 ns 7.5381 ns 7.5708 ns]
                        change: [+5.5214% +6.0313% +6.4757%] (p = 0.00 < 0.05)
                        Performance has regressed.
Benchmarking eq_in_page/String/16: Collecting 100 samples in estimated 5.0000 s (439
eq_in_page/String/16    time:   [10.089 ns 10.194 ns 10.345 ns]
                        change: [+9.1368% +10.153% +11.231%] (p = 0.00 < 0.05)
                        Performance has regressed.
Benchmarking eq_in_page/String/128: Collecting 100 samples in estimated 5.0000 s (31
eq_in_page/String/128   time:   [15.956 ns 16.020 ns 16.148 ns]
                        change: [+3.3239% +4.2398% +5.1960%] (p = 0.00 < 0.05)
                        Performance has regressed.
Benchmarking eq_in_page/String/512: Collecting 100 samples in estimated 5.0001 s (16
eq_in_page/String/512   time:   [29.182 ns 29.352 ns 29.569 ns]
                        change: [-1.7566% -0.9731% -0.3028%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Benchmarking eq_in_page/U32x2x2: Collecting 100 samples in estimated 5.0000 s (1.5B 
eq_in_page/U32x2x2      time:   [3.3729 ns 3.3938 ns 3.4353 ns]
                        change: [-88.097% -88.009% -87.929%] (p = 0.00 < 0.05)
                        Performance has improved.
Benchmarking eq_in_page/U32x4x2: Collecting 100 samples in estimated 5.0000 s (1.4B 
eq_in_page/U32x4x2      time:   [3.6449 ns 3.6607 ns 3.6853 ns]
                        change: [-92.001% -91.920% -91.870%] (p = 0.00 < 0.05)
                        Performance has improved.
Benchmarking eq_in_page/U32x2x4: Collecting 100 samples in estimated 5.0000 s (1.4B 
eq_in_page/U32x2x4      time:   [3.6469 ns 3.6907 ns 3.7494 ns]
                        change: [-92.797% -92.713% -92.629%] (p = 0.00 < 0.05)
                        Performance has improved.
Benchmarking eq_in_page/Option<U32x2>/None: Collecting 100 samples in estimated 5.00
eq_in_page/Option<U32x2>/None
                        time:   [3.3798 ns 3.3966 ns 3.4241 ns]
                        change: [-65.092% -63.961% -61.660%] (p = 0.00 < 0.05)
                        Performance has improved.
Benchmarking eq_in_page/Option<U32x2>/Some: Collecting 100 samples in estimated 5.00
eq_in_page/Option<U32x2>/Some
                        time:   [3.3756 ns 3.4034 ns 3.4457 ns]
                        change: [-81.915% -81.656% -81.353%] (p = 0.00 < 0.05)
                        Performance has improved.

API and ABI breaking changes

None

Expected complexity level and risk

2, fairly self-contained.

crates/table/src/eq.rs Outdated Show resolved Hide resolved
@Centril Centril requested a review from gefjon November 28, 2024 19:34
Copy link
Contributor

@gefjon gefjon left a comment

Choose a reason for hiding this comment

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

Possible correctness bug. Please convince me otherwise.

crates/table/src/eq.rs Outdated Show resolved Hide resolved
crates/table/src/layout.rs Show resolved Hide resolved
@Centril Centril force-pushed the centril/speedup-eq_row_in_page branch from 61ff04a to ca6ca19 Compare November 30, 2024 02:16
@Centril Centril requested a review from gefjon November 30, 2024 02:16
@gefjon
Copy link
Contributor

gefjon commented Dec 2, 2024

Please rebase or merge on top of 80dff96 (#1909 ); that should fix the internal tests.

@bfops bfops added the release-any To be landed in any release window label Dec 2, 2024
Use this for a fast path in `eq_row_in_page` & `row_type_visitor`.
@Centril Centril force-pushed the centril/speedup-eq_row_in_page branch from ca6ca19 to a77b83d Compare December 3, 2024 14:51
@Centril Centril enabled auto-merge December 3, 2024 14:52
@Centril Centril added this pull request to the merge queue Dec 3, 2024
Merged via the queue into master with commit bc71e25 Dec 3, 2024
8 checks passed
@Centril Centril deleted the centril/speedup-eq_row_in_page branch December 3, 2024 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-any To be landed in any release window
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants