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

Default RERUN_CHUNK_MAX_BYTES to 384kiB instead of 4MiB #7263

Merged
merged 2 commits into from
Aug 26, 2024

Conversation

teh-cmc
Copy link
Member

@teh-cmc teh-cmc commented Aug 24, 2024

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested the web demo (if applicable):
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
  • If applicable, add a new check to the release checklist!
  • If have noted any breaking changes to the log API in CHANGELOG.md and the migration guide

To run all checks from main, comment on the PR with @rerun-bot full-check.

@teh-cmc teh-cmc added ⛃ re_datastore affects the datastore itself 📺 re_viewer affects re_viewer itself 🚀 performance Optimization, memory use, etc include in changelog labels Aug 24, 2024
@nikolausWest
Copy link
Member

Didn’t we have increase this to speed up time series?

@teh-cmc
Copy link
Member Author

teh-cmc commented Aug 24, 2024

Didn’t we have increase this to speed up time series?

This does not affect time series since 4096 rows (RERUN_CHUNK_MAX_ROWS) of scalars still fit into 256kiB (RERUN_CHUNK_MAX_BYTES) (even when accounting for timelines, row ids, offsets, etc), with more or less margin depending on how many timelines are used.

I can probably bump it to 12 * 8 * 4096 bytes to give users even more leeway without any negative impact on the problematic workloads though. I'll try it.

@teh-cmc teh-cmc changed the title Default RERUN_CHUNK_MAX_BYTES to 256kiB instead of 4MiB Default RERUN_CHUNK_MAX_BYTES to 384kiB instead of 4MiB Aug 24, 2024
@teh-cmc
Copy link
Member Author

teh-cmc commented Aug 24, 2024

I'll try it.

Works fine.

@emilk emilk added this to the 0.18.1 milestone Aug 26, 2024
@emilk emilk merged commit 20da873 into main Aug 26, 2024
35 checks passed
@emilk emilk deleted the cmc/smaller_chunk_bytes_threshold branch August 26, 2024 06:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
include in changelog 🚀 performance Optimization, memory use, etc ⛃ re_datastore affects the datastore itself 📺 re_viewer affects re_viewer itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Significant performance drop after update from 0.16 to 0.18
3 participants