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.

Sign up for GitHub

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

Jump to bottom

DataRow.total_size_bytes is wrong, so RERUN_FLUSH_NUM_BYTES doesn't work, leading to huge messages in our example .rrds #5078

Closed
emilk opened this issue Feb 7, 2024 · 1 comment · Fixed by #5086
Closed

DataRow.total_size_bytes is wrong, so RERUN_FLUSH_NUM_BYTES doesn't work, leading to huge messages in our example .rrds #5078

emilk opened this issue Feb 7, 2024 · 1 comment · Fixed by #5086
Assignees
@emilk
Labels
😤 annoying Something in the UI / SDK is annoying to use 🏎️ Quick Issue Can be fixed in a few hours or less

Comments

@emilk
Copy link
Member

emilk commented Feb 7, 2024

For instance https://app.rerun.io/version/nightly/examples/nuscenes.rrd consists of only three arrow messages, of size 90.6 MiB, 12 MiB and 244 KiB. We should be able to break that down in much smaller chunks.

crates/re_build_examples/src/rrd.rs does set RERUN_FLUSH_NUM_BYTES to 128 KiB, so I'm not sure what is going on here.
@emilk emilk added 😤 annoying Something in the UI / SDK is annoying to use 👀 needs triage This issue needs to be triaged by the Rerun team 🏎️ Quick Issue Can be fixed in a few hours or less and removed 👀 needs triage This issue needs to be triaged by the Rerun team labels Feb 7, 2024
@emilk emilk mentioned this issue Feb 7, 2024
Merged
5 tasks
@emilk emilk self-assigned this Feb 7, 2024
@emilk
Copy link
Member Author

emilk commented Feb 7, 2024

It's row.total_size_bytes() in crates/re_log_types/src/data_table_batcher.rs that is completely wrong. The batcher thinks it is processing kilobytes when it is processing megabytes.

@emilk emilk changed the title Example rrd don't stream well DataRow.total_size_bytes is wrong, so RERUN_FLUSH_NUM_BYTES doesn't work, leading to huge messages in our example .rrds Feb 7, 2024
emilk added a commit that referenced this issue Feb 7, 2024
@emilk
rerun print: print just summary, unless given --verbose (#5079)
dd18b03 
### What
Added when debugging #5078

Example:

```
> rerun print nuscenes.rrd

StoreInfo {
    application_id: ApplicationId(
        "rerun_example_nuscenes",
    ),
    store_id: StoreId {
        kind: Recording,
        id: "58821cc9-d39c-4f78-9e7e-bad4933e9870",
    },
    is_official_example: true,
    started: "2024-02-06 12:34:04.306130Z",
    store_source: PythonSdk(
        3.11.6,
    ),
    store_kind: Recording,
}
StoreInfo {
    application_id: ApplicationId(
        "rerun_example_nuscenes",
    ),
    store_id: StoreId {
        kind: Recording,
        id: "58821cc9-d39c-4f78-9e7e-bad4933e9870",
    },
    is_official_example: true,
    started: "2024-02-06 12:34:04.306130Z",
    store_source: PythonSdk(
        3.11.6,
    ),
    store_kind: Recording,
}
Table with 596 rows (90.6 MiB). Columns: ["Color", "ImageIndicator", "InstanceKey", "PinholeIndicator", "PinholeProjection", "Points3DIndicator", "Position3D", "Resolution", "TensorData", "Transform3D", "Transform3DIndicator", "ViewCoordinates", "ViewCoordinatesIndicator"]
Table with 586 rows (12.0 MiB). Columns: ["Color", "ImageIndicator", "InstanceKey", "Points3DIndicator", "Position3D", "TensorData"]
Table with 179 rows (244 KiB). Columns: ["Boxes3DIndicator", "ClassId", "Color", "HalfSizes3D", "InstanceKey", "Points3DIndicator", "Position3D", "Rotation3D"]
```

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using newly built examples:
[app.rerun.io](https://app.rerun.io/pr/5079/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/5079/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[app.rerun.io](https://app.rerun.io/pr/5079/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/5079)
- [Docs
preview](https://rerun.io/preview/5077a74742e41561744cc03c190dfe8da149559d/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/5077a74742e41561744cc03c190dfe8da149559d/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
@emilk emilk mentioned this issue Feb 7, 2024
Merged
5 tasks
emilk added a commit that referenced this issue Feb 7, 2024
@emilk
Fix RERUN_FLUSH_NUM_BYTES and data size estimations (#5086)
e0f51ab 
### What
* Closes #5078

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
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using newly built examples:
[app.rerun.io](https://app.rerun.io/pr/5086/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/5086/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[app.rerun.io](https://app.rerun.io/pr/5086/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/5086)
- [Docs
preview](https://rerun.io/preview/1830ed9e7665f8c067fbf3ea9192afff1cc100e8/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/1830ed9e7665f8c067fbf3ea9192afff1cc100e8/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@emilk emilk

Labels
😤 annoying Something in the UI / SDK is annoying to use 🏎️ Quick Issue Can be fixed in a few hours or less
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix RERUN_FLUSH_NUM_BYTES and data size estimations rerun-io/rerun
1 participant
@emilk