Skip to content

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.

@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.

@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.

4 participants