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

Fix RERUN_FLUSH_NUM_BYTES and data size estimations #5086

Merged
merged 10 commits into from Feb 7, 2024

Conversation

emilk
Copy link
Member

@emilk emilk commented Feb 7, 2024

What

The first commit is the actual fix: the size calculations were completely wrong for fixed-size array and SmallVec, leading the batcher to severely under-counting how many bytes it had processed, leading it to effectively ignore RERUN_FLUSH_NUM_BYTES , which in turn lead to example .rrds which big batches of data, which streams poorly.

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!

@emilk emilk added 🪳 bug Something isn't working 😤 annoying Something in the UI / SDK is annoying to use labels Feb 7, 2024
@emilk emilk changed the title Emilk/fix size flushing Fix RERUN_FLUSH_NUM_BYTES and data size estimations Feb 7, 2024
Copy link

github-actions bot commented Feb 7, 2024

Size changes

Name main 5086/merge Change
dna.rrd 552.96 kiB 655.36 kiB +18.52%
plots.rrd 1054.72 kiB 163.84 kiB -84.47%
structure_from_motion.rrd 6.51 MiB 6.84 MiB +5.07%

@emilk emilk marked this pull request as ready for review February 7, 2024 11:24
@emilk
Copy link
Member Author

emilk commented Feb 7, 2024

⬆ this means it is working!

@teh-cmc teh-cmc self-requested a review February 7, 2024 11:26
@emilk emilk merged commit e0f51ab into main Feb 7, 2024
40 of 41 checks passed
@emilk emilk deleted the emilk/fix-size-flushing branch February 7, 2024 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
😤 annoying Something in the UI / SDK is annoying to use 🪳 bug Something isn't working include in changelog
Projects
None yet
2 participants