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

Create matching benchmarks for the arrow datastore #582

Merged
merged 8 commits into from
Dec 18, 2022
Merged

Conversation

jleibs
Copy link
Member

@jleibs jleibs commented Dec 16, 2022

If these look terrifying, don't panic. Improvements are coming in a followup PR:

obj_mono_points/insert  time:   [107.87 ms 108.56 ms 109.41 ms]
                        thrpt:  [91.400 Kelem/s 92.118 Kelem/s 92.700 Kelem/s]
                 change:
                        time:   [+5529.6% +5637.2% +5741.8%] (p = 0.00 < 0.05)
                        thrpt:  [-98.288% -98.257% -98.224%]
                        Performance has regressed.

obj_mono_points/query   time:   [21.120 ms 21.316 ms 21.533 ms]
                        thrpt:  [4.6440 Kelem/s 4.6914 Kelem/s 4.7348 Kelem/s]
                 change:
                        time:   [+229611% +232068% +234698%] (p = 0.00 < 0.05)
                        thrpt:  [-99.957% -99.957% -99.956%]
                        Performance has regressed.

obj_batch_points/insert time:   [492.58 µs 493.57 µs 494.80 µs]
                        thrpt:  [20.210 Melem/s 20.261 Melem/s 20.301 Melem/s]
                 change:
                        time:   [+31.371% +31.914% +32.426%] (p = 0.00 < 0.05)
                        thrpt:  [-24.486% -24.193% -23.880%]
                        Performance has regressed.

obj_batch_points/query  time:   [285.54 µs 290.11 µs 295.94 µs]
                        thrpt:  [337.91 Kelem/s 344.70 Kelem/s 350.22 Kelem/s]
                 change:
                        time:   [+43136% +44278% +45463%] (p = 0.00 < 0.05)
                        thrpt:  [-99.781% -99.775% -99.769%]
                        Performance has regressed.

Resolves: #575

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I've added a line to CHANGELOG.md (if this is a big enough change to warrant it)

@jleibs jleibs marked this pull request as ready for review December 16, 2022 16:01
Base automatically changed from jleibs/minimal_arrow_ui to main December 16, 2022 16:05
@jleibs jleibs force-pushed the jleibs/arrow_bench branch from 5e2338b to 53dfd61 Compare December 16, 2022 16:07
Copy link
Member

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Awesome to finally get these in!

Comment on lines +18 to +21
#[cfg(not(debug_assertions))]
const NUM_FRAMES: u32 = 100;
#[cfg(not(debug_assertions))]
const NUM_POINTS: u32 = 100;
Copy link
Member

Choose a reason for hiding this comment

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

in crates/re_data_store/benches/obj_query_benchmark.rs we use 1000 for both these. I guess arrow is still too slow for that?

It would be nice to have matching numbers so we can compare apples to apples

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup -- this is coming as soon as we land #582

.into_iter()
.map(move |point_idx| obj_path!("points", Index::Sequence(point_idx as _)))
.collect_vec();
let msgs = build_messages(&paths, 1);
Copy link
Member

Choose a reason for hiding this comment

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

unrelated, but message encoding/decoding is another thing we would like to benchmark!

crates/re_query/benches/obj_query_benchmark.rs Outdated Show resolved Hide resolved
@teh-cmc teh-cmc self-requested a review December 18, 2022 00:29
@jleibs jleibs merged commit 1786633 into main Dec 18, 2022
@jleibs jleibs deleted the jleibs/arrow_bench branch December 18, 2022 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create benchmark for new re_query crate
2 participants