Skip to content

Default RERUN_CHUNK_MAX_BYTES to 384kiB instead of 4MiB#7263

Merged
emilk merged 2 commits intomainfrom
cmc/smaller_chunk_bytes_threshold
Aug 26, 2024
Merged

Default RERUN_CHUNK_MAX_BYTES to 384kiB instead of 4MiB#7263
emilk merged 2 commits intomainfrom
cmc/smaller_chunk_bytes_threshold

Conversation

@teh-cmc
Copy link
Copy Markdown
Contributor

@teh-cmc teh-cmc commented Aug 24, 2024 •
edited by github-actions bot
Loading

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
Copy Markdown
Member

nikolausWest commented Aug 24, 2024

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

@teh-cmc
Copy link
Copy Markdown
Contributor 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
Copy Markdown
Contributor 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
@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

Reviewers

@emilk emilk emilk approved these changes

Assignees

No one assigned

Labels

include in changelog 📉 performance Optimization, memory use, etc ⛃ re_datastore affects the datastore itself 📺 re_viewer affects re_viewer itself

Projects

None yet

Milestone

0.18.1

Development

Successfully merging this pull request may close these issues.

Significant performance drop after update from 0.16 to 0.18

3 participants

@teh-cmc @nikolausWest @emilk