Skip to content

Commit

Permalink
Use rerun::Collection almost everywhere we'd use std::vector befo…
Browse files Browse the repository at this point in the history
…re (#4247)

* Part of the series of #3794 

### What
... and various improvements to `rerun::Collection` in general to be up
for the task. Added copy assignment/construction, added vector
conversion, removed serialization method (handled at callsites now)

Also, I overhauled and generalized our builtin
`rerun::CollectionAdapters` further. They give now more exact guarantees
on when they copy & move data.
To support the considerably increased significance and abilitites of
this data structure I added a lot more tests, in particular checking on
the aforementioned move/copy guarantees.

Huge thanks to @AnkeAnke for pointing out to me how we can support std
compatible contains generically in a trivial way
(`rerun::type_traits::value_type_t`!). This made it possible that this
PR requires almost no changes in user/test/example code since most types
just snap in the same as before.

As expected this change improves the performance on the image logging
benchmark (which logs 4 16k rgba images).
Timing on M1 Max went from ~2.9s to ~2.4s and on the AMD windows box I
use from ~3.0s to ~2.5s
(did a bunch of runs, values are excluding prepare and process setup,
AMD fluctuates a lot)


DRAFT until I'm sure this works on more compilers.

### 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 [demo.rerun.io](https://demo.rerun.io/pr/4247) (if
applicable)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4247)
- [Docs
preview](https://rerun.io/preview/825bf7905ccaef0f23d87a4ffe0ae406018edffa/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/825bf7905ccaef0f23d87a4ffe0ae406018edffa/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
  • Loading branch information
Wumpf authored Nov 17, 2023
1 parent cb85313 commit 907d006
Show file tree
Hide file tree
Showing 113 changed files with 2,555 additions and 1,653 deletions.
70 changes: 45 additions & 25 deletions crates/re_types_builder/src/codegen/cpp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ impl QuotedObject {
let method_ident = format_ident!("with_{}", obj_field.name);
let field_type = quote_archetype_field_type(&mut hpp_includes, obj_field);

hpp_includes.insert_rerun("warning_macros.hpp");
hpp_includes.insert_rerun("compiler_utils.hpp");
let gcc_ignore_comment =
quote_comment("See: https://github.com/rerun-io/rerun/issues/4027");

Expand All @@ -433,7 +433,7 @@ impl QuotedObject {
#field_ident = std::move(#parameter_ident);
#NEWLINE_TOKEN
#gcc_ignore_comment
WITH_MAYBE_UNINITIALIZED_DISABLED(return std::move(*this);)
RERUN_WITH_MAYBE_UNINITIALIZED_DISABLED(return std::move(*this);)
},
inline: true,
});
Expand Down Expand Up @@ -1343,41 +1343,60 @@ fn component_to_data_cell_method(type_ident: &Ident, hpp_includes: &mut Includes
fn archetype_serialize(type_ident: &Ident, obj: &Object, hpp_includes: &mut Includes) -> Method {
hpp_includes.insert_rerun("data_cell.hpp");
hpp_includes.insert_rerun("collection.hpp");
hpp_includes.insert_rerun("serialized_component_batch.hpp");
hpp_includes.insert_system("vector"); // std::vector

let num_fields = quote_integer(obj.fields.len());
let push_batches = obj.fields.iter().map(|field| {
let field_name = format_ident!("{}", field.name);
let field_accessor = quote!(archetype.#field_name);
let field_type = quote_fqname_as_type_path(
hpp_includes,
field
.typ
.fqname()
.expect("Archetypes only have components and vectors of components."),
);

// TODO(andreas): Introducing MonoCollection will remove the need for this.
let wrapping_type = if field.typ.is_plural() {
quote!()
} else {
let field_type = quote_fqname_as_type_path(
hpp_includes,
field
.typ
.fqname()
.expect("Archetypes only have components and vectors of components."),
);
quote!(Collection<#field_type>)
let emplace_back = quote! {
RR_RETURN_NOT_OK(result.error);
cells.emplace_back(std::move(result.value), size);
};

if field.is_nullable {

// TODO(andreas): Introducing MonoCollection will remove the need for distinguishing these two cases.
if field.typ.is_plural() {
if field.is_nullable {
quote! {
if (#field_accessor.has_value()) {
const size_t size = #field_accessor.value().size();
auto result = #field_type::to_data_cell(#field_accessor.value().data(), size);
#emplace_back
}
}
} else {
quote! {
{
const size_t size = #field_accessor.size();
auto result = #field_type::to_data_cell(#field_accessor.data(), #field_accessor.size());
#emplace_back
}
}
}
} else if field.is_nullable {
quote! {
if (#field_accessor.has_value()) {
auto result = #wrapping_type(#field_accessor.value()).serialize();
RR_RETURN_NOT_OK(result.error);
cells.emplace_back(std::move(result.value));
const size_t size = 1;
auto result = #field_type::to_data_cell(&#field_accessor.value(), size);
#emplace_back
}
}
} else {
quote! {
{
auto result = #wrapping_type(#field_accessor).serialize();
RR_RETURN_NOT_OK(result.error);
cells.emplace_back(std::move(result.value));
const size_t size = 1;
auto result = #field_type::to_data_cell(&#field_accessor, size);
#emplace_back
}
}
}
Expand All @@ -1399,9 +1418,10 @@ fn archetype_serialize(type_ident: &Ident, obj: &Object, hpp_includes: &mut Incl
#NEWLINE_TOKEN
#(#push_batches)*
{
auto result = Collection<#type_ident::IndicatorComponent>(#type_ident::IndicatorComponent()).serialize();
auto indicator = #type_ident::IndicatorComponent();
auto result = #type_ident::IndicatorComponent::to_data_cell(&indicator, 1);
RR_RETURN_NOT_OK(result.error);
cells.emplace_back(std::move(result.value));
cells.emplace_back(std::move(result.value), 1);
}
#NEWLINE_TOKEN
#NEWLINE_TOKEN
Expand Down Expand Up @@ -1983,8 +2003,8 @@ fn quote_field_type(includes: &mut Includes, obj_field: &ObjectField) -> TokenSt
}
Type::Vector { elem_type } => {
let elem_type = quote_element_type(includes, elem_type);
includes.insert_system("vector");
quote! { std::vector<#elem_type> }
includes.insert_rerun("collection.hpp");
quote! { rerun::Collection<#elem_type> }
}
Type::Object(fqname) => {
let type_name = quote_fqname_as_type_path(includes, fqname);
Expand Down
4 changes: 2 additions & 2 deletions docs/code-examples/line_strip2d_batch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ int main() {
const auto rec = rerun::RecordingStream("rerun_example_line_strip2d");
rec.spawn().exit_on_failure();

std::vector<rerun::Vec2D> strip1 = {{0.f, 0.f}, {2.f, 1.f}, {4.f, -1.f}, {6.f, 0.f}};
std::vector<rerun::Vec2D> strip2 =
rerun::Collection<rerun::Vec2D> strip1 = {{0.f, 0.f}, {2.f, 1.f}, {4.f, -1.f}, {6.f, 0.f}};
rerun::Collection<rerun::Vec2D> strip2 =
{{0.f, 3.f}, {1.f, 4.f}, {2.f, 2.f}, {3.f, 4.f}, {4.f, 2.f}, {5.f, 4.f}, {6.f, 3.f}};
rec.log(
"strips",
Expand Down
4 changes: 2 additions & 2 deletions docs/code-examples/line_strip3d_batch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ int main() {
const auto rec = rerun::RecordingStream("rerun_example_line_strip3d");
rec.spawn().exit_on_failure();

std::vector<rerun::Vec3D> strip1 = {
rerun::Collection<rerun::Vec3D> strip1 = {
{0.f, 0.f, 2.f},
{1.f, 0.f, 2.f},
{1.f, 1.f, 2.f},
{0.f, 1.f, 2.f},
};
std::vector<rerun::Vec3D> strip2 = {
rerun::Collection<rerun::Vec3D> strip2 = {
{0.f, 0.f, 0.f},
{0.f, 0.f, 1.f},
{1.f, 0.f, 0.f},
Expand Down
2 changes: 2 additions & 0 deletions rerun_cpp/docs/writing_docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,5 @@ We stick to the following styles:
/// \endcond
```
* Avoid the use of groups when namespaces can be used instead
* Don't omit namespaces when referring to types in docs - instead of `Collection` use `rerun::Collection`.
Both works usually but the latter makes it easier to understand what is meant.
13 changes: 6 additions & 7 deletions rerun_cpp/src/rerun/archetypes/annotation_context.cpp

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions rerun_cpp/src/rerun/archetypes/annotation_context.hpp

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

52 changes: 35 additions & 17 deletions rerun_cpp/src/rerun/archetypes/arrows3d.cpp

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 8 additions & 7 deletions rerun_cpp/src/rerun/archetypes/arrows3d.hpp

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 15 additions & 11 deletions rerun_cpp/src/rerun/archetypes/asset3d.cpp

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 907d006

Please sign in to comment.