Skip to content

Commit

Permalink
C++ codegen extensions, archetype tests, array ctors (#2916)
Browse files Browse the repository at this point in the history
### What

Introduces a simple extension system for C++ codegen: Add an extra cpp
file that will be compiled as part of the SDK. A section between two
markes is copied into the generated hpp as part of generation (typically
this section is removed from compilation via #ifdef)

Adds extensions to:
* color
* vec2/vec3/vec4
* quaternion
* origin3d
* point2d
* point3d
* arrow3d

... and uses them to simplify examples and test code!

All fully supported archetypes now have a simple test that checks that
the base interface works and that we can serialize out to arrow without
issues.

Additionally, there's tests for vecN/quaternion/color to check that
their various constructors work as expected (not being a C++ expert it's
fairly hard to predict)

Extends codegen with single-array-field-constructors.


---------

* part of #2647
* Fixes #2798
* Fixes #2785
* Missing only 2D example: #2789
* Adds to #2791 


### 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/2916) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/2916)
- [Docs
preview](https://rerun.io/preview/pr%3Aandreas%2Fcpp%2Fcustom-extensions/docs)
- [Examples
preview](https://rerun.io/preview/pr%3Aandreas%2Fcpp%2Fcustom-extensions/examples)
  • Loading branch information
Wumpf authored Aug 7, 2023
1 parent b63f875 commit 52d66ea
Show file tree
Hide file tree
Showing 49 changed files with 1,143 additions and 118 deletions.
48 changes: 42 additions & 6 deletions crates/re_build_tools/src/hashing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,24 +25,33 @@ pub fn iter_dir<'a>(
path: impl AsRef<Path>,
extensions: Option<&'a [&'a str]>,
) -> impl Iterator<Item = PathBuf> + 'a {
fn filter(entry: &walkdir::DirEntry, extensions: Option<&[&str]>) -> bool {
let is_dir = entry.file_type().is_dir();
let is_interesting = extensions.map_or(true, |extensions| {
iter_dir_filtered(path, move |entry: &Path| {
extensions.map_or(true, |extensions| {
extensions.iter().any(|ext| {
entry
.path()
.extension()
.map_or(false, |ext2| *ext == ext2.to_string_lossy())
})
});
})
})
}

/// Recursively walks the directory at `path` in filename order with an arbitrary filter.
pub fn iter_dir_filtered<'a>(
path: impl AsRef<Path>,
custom_filter: impl Fn(&Path) -> bool + 'a,
) -> impl Iterator<Item = PathBuf> + 'a {
fn filter(entry: &walkdir::DirEntry, custom_filter: &impl Fn(&Path) -> bool) -> bool {
let is_dir = entry.file_type().is_dir();
let is_interesting = custom_filter(entry.path());
is_dir || is_interesting
}

let path = path.as_ref();
walkdir::WalkDir::new(path)
.sort_by_file_name()
.into_iter()
.filter_entry(move |entry| filter(entry, extensions))
.filter_entry(move |entry| filter(entry, &custom_filter))
.filter_map(|entry| entry.ok())
.filter_map(|entry| entry.file_type().is_file().then(|| entry.into_path()))
}
Expand Down Expand Up @@ -96,6 +105,33 @@ pub fn compute_dir_hash<'a>(path: impl AsRef<Path>, extensions: Option<&'a [&'a
encode_hex(hasher.finalize().as_slice())
}

/// Given a directory path, computes the sha256 hash of the accumulated contents of all of its
/// files (ordered by filename) except those failing a a custom filter, and returns an hexadecimal string for it.
///
/// This includes files in sub-directories (i.e. it's recursive).
///
/// This will automatically emit a `rerun-if-changed` clause for all the files that were hashed.
pub fn compute_dir_filtered_hash<'a>(
path: impl AsRef<Path>,
custom_filter: impl Fn(&Path) -> bool + 'a,
) -> String {
let mut hasher = Sha256::new();

let path = path.as_ref();
for filepath in iter_dir_filtered(path, custom_filter) {
let mut file = fs::File::open(&filepath)
.with_context(|| format!("couldn't open {filepath:?}"))
.unwrap();
io::copy(&mut file, &mut hasher)
.with_context(|| format!("couldn't copy from {filepath:?}"))
.unwrap();

rerun_if_changed(filepath);
}

encode_hex(hasher.finalize().as_slice())
}

/// Given a crate name, computes the sha256 hash of its source code (ordered by filename) and
/// returns an hexadecimal string for it.
///
Expand Down
4 changes: 2 additions & 2 deletions crates/re_build_tools/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ mod rebuild_detector;
pub(crate) use self::rebuild_detector::Packages;

pub use self::hashing::{
compute_crate_hash, compute_dir_hash, compute_file_hash, compute_strings_hash, iter_dir,
read_versioning_hash, write_versioning_hash,
compute_crate_hash, compute_dir_filtered_hash, compute_dir_hash, compute_file_hash,
compute_strings_hash, iter_dir, read_versioning_hash, write_versioning_hash,
};
pub use self::rebuild_detector::{
get_and_track_env_var, is_tracked_env_var_set, rebuild_if_crate_changed, rerun_if_changed,
Expand Down
31 changes: 18 additions & 13 deletions crates/re_types/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
use std::path::PathBuf;

use re_build_tools::{
compute_crate_hash, compute_dir_hash, compute_strings_hash, is_tracked_env_var_set, iter_dir,
read_versioning_hash, rerun_if_changed, rerun_if_changed_or_doesnt_exist,
write_versioning_hash,
compute_crate_hash, compute_dir_filtered_hash, compute_dir_hash, compute_strings_hash,
is_tracked_env_var_set, iter_dir, read_versioning_hash, rerun_if_changed,
rerun_if_changed_or_doesnt_exist, write_versioning_hash,
};

// ---
Expand Down Expand Up @@ -49,36 +49,41 @@ fn main() {
let cur_hash = read_versioning_hash(SOURCE_HASH_PATH);
let re_types_builder_hash = compute_crate_hash("re_types_builder");
let definitions_hash = compute_dir_hash(DEFINITIONS_DIR_PATH, Some(&["fbs"]));
let doc_examples_hash = compute_dir_hash(DOC_EXAMPLES_DIR_PATH, Some(&["rs", "py"]));
let archetype_overrides_hash = compute_dir_hash(
let doc_examples_hash = compute_dir_hash(DOC_EXAMPLES_DIR_PATH, Some(&["rs", "py", "cpp"]));
let python_archetype_overrides_hash = compute_dir_hash(
PathBuf::from(PYTHON_OUTPUT_DIR_PATH).join(ARCHETYPE_OVERRIDES_SUB_DIR_PATH),
Some(&["py"]),
);
let component_overrides_hash = compute_dir_hash(
let python_component_overrides_hash = compute_dir_hash(
PathBuf::from(PYTHON_OUTPUT_DIR_PATH).join(COMPONENT_OVERRIDES_SUB_DIR_PATH),
Some(&["py"]),
);
let datatype_overrides_hash = compute_dir_hash(
let python_datatype_overrides_hash = compute_dir_hash(
PathBuf::from(PYTHON_OUTPUT_DIR_PATH).join(DATATYPE_OVERRIDES_SUB_DIR_PATH),
Some(&["py"]),
);
let cpp_extensions_hash = compute_dir_filtered_hash(CPP_OUTPUT_DIR_PATH, |path| {
path.to_str().unwrap().ends_with("_ext.cpp")
});

let new_hash = compute_strings_hash(&[
&re_types_builder_hash,
&definitions_hash,
&doc_examples_hash,
&archetype_overrides_hash,
&component_overrides_hash,
&datatype_overrides_hash,
&python_archetype_overrides_hash,
&python_component_overrides_hash,
&python_datatype_overrides_hash,
&cpp_extensions_hash,
]);

// Leave these be please, very useful when debugging.
eprintln!("re_types_builder_hash: {re_types_builder_hash:?}");
eprintln!("definitions_hash: {definitions_hash:?}");
eprintln!("doc_examples_hash: {doc_examples_hash:?}");
eprintln!("archetype_overrides_hash: {archetype_overrides_hash:?}");
eprintln!("component_overrides_hash: {component_overrides_hash:?}");
eprintln!("datatype_overrides_hash: {datatype_overrides_hash:?}");
eprintln!("python_archetype_overrides_hash: {python_archetype_overrides_hash:?}");
eprintln!("python_component_overrides_hash: {python_component_overrides_hash:?}");
eprintln!("python_datatype_overrides_hash: {python_datatype_overrides_hash:?}");
eprintln!("cpp_extensions_hash: {cpp_extensions_hash:?}");
eprintln!("new_hash: {new_hash:?}");
eprintln!("cur_hash: {cur_hash:?}");

Expand Down
2 changes: 1 addition & 1 deletion crates/re_types/source_hash.txt

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

14 changes: 14 additions & 0 deletions crates/re_types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,20 @@
//! This sibling file needs to implement an extension class that is mixed in with the
//! auto-generated class.
//! The simplest way to get started is to look at any of the existing examples.
//!
//!
//! #### C++
//!
//! Generated C++ code can be manually extended by adding a sibling file with the `_ext.cpp` suffix.
//! E.g. to extend `vec2d.cpp`, create a `vec2d_ext.cpp`.
//!
//! The sibling file is compiled as-is as part of the `rerun_cpp` crate.
//! In order to extend the generated type declaration in the header,
//! you can specify a single code-block that you want to be injected into the type declaration by
//! starting it with `[CODEGEN COPY TO HEADER START]` and ending it with `[CODEGEN COPY TO HEADER END]`.
//! Note that it is your responsibility to make sure that the cpp file is valid C++ code -
//! the code generator & build will not adjust the extension file for you!
//!

// ---

Expand Down
2 changes: 1 addition & 1 deletion crates/re_types_builder/src/codegen/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ pub fn remove_old_files_from_folder(folder_path: Utf8PathBuf, filepaths: &BTreeS
re_tracing::profile_function!();
for entry in std::fs::read_dir(folder_path).unwrap().flatten() {
let filepath = Utf8PathBuf::try_from(entry.path()).unwrap();
if filepath.as_str().ends_with("_ext.rs") {
if filepath.as_str().ends_with("_ext.rs") || filepath.as_str().ends_with("_ext.cpp") {
continue;
}
if !filepaths.contains(&filepath) {
Expand Down
106 changes: 95 additions & 11 deletions crates/re_types_builder/src/codegen/cpp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ const DOC_COMMENT_PREFIX_TOKEN: &str = "DOC_COMMENT_PREFIX_TOKEN";
const DOC_COMMENT_SUFFIX_TOKEN: &str = "DOC_COMMENT_SUFFIX_TOKEN";
const SYS_INCLUDE_PATH_PREFIX_TOKEN: &str = "SYS_INCLUDE_PATH_PREFIX_TOKEN";
const SYS_INCLUDE_PATH_SUFFIX_TOKEN: &str = "SYS_INCLUDE_PATH_SUFFIX_TOKEN";
const HEADER_EXTENSION_PREFIX_TOKEN: &str = "HEADER_EXTENSION_PREFIX_TOKEN";
const HEADER_EXTENSION_SUFFIX_TOKEN: &str = "HEADER_EXTENSION_SUFFIX_TOKEN";
const TODO_TOKEN: &str = "TODO_TOKEN";

fn quote_comment(text: &str) -> TokenStream {
Expand All @@ -56,10 +58,13 @@ fn string_from_token_stream(token_stream: &TokenStream, source_path: Option<&Utf
&token_stream
.to_string()
.replace(&format!("{NEWLINE_TOKEN:?}"), "\n")
.replace(NEWLINE_TOKEN, "\n") // Should only happen inside header extensions.
.replace(&format!("{NORMAL_COMMENT_PREFIX_TOKEN:?} \""), "//")
.replace(&format!("\" {NORMAL_COMMENT_SUFFIX_TOKEN:?}"), "\n")
.replace(&format!("{DOC_COMMENT_PREFIX_TOKEN:?} \""), "///")
.replace(&format!("\" {DOC_COMMENT_SUFFIX_TOKEN:?}"), "\n")
.replace(&format!("{HEADER_EXTENSION_PREFIX_TOKEN:?} \""), "")
.replace(&format!("\" {HEADER_EXTENSION_SUFFIX_TOKEN:?}"), "")
.replace(&format!("{SYS_INCLUDE_PATH_PREFIX_TOKEN:?} \""), "<")
.replace(&format!("\" {SYS_INCLUDE_PATH_SUFFIX_TOKEN:?}"), ">")
.replace(
Expand Down Expand Up @@ -106,7 +111,10 @@ impl CppCodeGenerator {
let ordered_objects = objects.ordered_objects(object_kind.into());
for &obj in &ordered_objects {
let filename = obj.snake_case_name();
let (hpp, cpp) = generate_hpp_cpp(objects, arrow_registry, obj);
let extension_filename = folder_path.join(format!("{filename}_ext.cpp"));
let hpp_type_extensions = hpp_type_extensions(&extension_filename);

let (hpp, cpp) = generate_hpp_cpp(objects, arrow_registry, obj, &hpp_type_extensions);
for (extension, tokens) in [("hpp", hpp), ("cpp", cpp)] {
let string = string_from_token_stream(&tokens, obj.relative_filepath());
let filepath = folder_path.join(format!("{filename}.{extension}"));
Expand Down Expand Up @@ -163,12 +171,50 @@ impl crate::CodeGenerator for CppCodeGenerator {
}
}

/// Retrieves code from an extension cpp file that should go to the generated header.
fn hpp_type_extensions(extension_file: &Utf8Path) -> TokenStream {
let Ok(content) = std::fs::read_to_string(extension_file.as_std_path()) else {
return quote! {};
};

const COPY_TO_HEADER_START_MARKER: &str = "[CODEGEN COPY TO HEADER START]";
const COPY_TO_HEADER_END_MARKER: &str = "[CODEGEN COPY TO HEADER END]";

let start = content.find(COPY_TO_HEADER_START_MARKER).unwrap_or_else(||
panic!("C++ extension file missing start marker. Without it, nothing is exposed to the header, i.e. not accessible to the user. Expected to find '{COPY_TO_HEADER_START_MARKER}' in {extension_file:?}")
);

let end = content.find(COPY_TO_HEADER_END_MARKER).unwrap_or_else(||
panic!("C++ extension file has a start marker but no end marker. Expected to find '{COPY_TO_HEADER_START_MARKER}' in {extension_file:?}")
);
let end = content[..end].rfind('\n').unwrap_or_else(||
panic!("Expected line break at some point before {COPY_TO_HEADER_END_MARKER} in {extension_file:?}")
);

let comment = quote_comment(&format!(
"Extensions to generated type defined in '{}'",
extension_file.file_name().unwrap()
));
let extensions = &content[start + COPY_TO_HEADER_START_MARKER.len()..end]
.replace('\n', &format!(" {NEWLINE_TOKEN} "));
quote! {
public:
#NEWLINE_TOKEN
#comment
#NEWLINE_TOKEN
#HEADER_EXTENSION_PREFIX_TOKEN #extensions #HEADER_EXTENSION_SUFFIX_TOKEN
#NEWLINE_TOKEN
}
}

fn generate_hpp_cpp(
objects: &Objects,
arrow_registry: &ArrowRegistry,
obj: &Object,
hpp_type_extensions: &TokenStream,
) -> (TokenStream, TokenStream) {
let QuotedObject { hpp, cpp } = QuotedObject::new(arrow_registry, objects, obj);
let QuotedObject { hpp, cpp } =
QuotedObject::new(arrow_registry, objects, obj, hpp_type_extensions);
let snake_case_name = obj.snake_case_name();
let hash = quote! { # };
let pragma_once = pragma_once();
Expand Down Expand Up @@ -199,17 +245,27 @@ struct QuotedObject {
}

impl QuotedObject {
pub fn new(arrow_registry: &ArrowRegistry, objects: &Objects, obj: &Object) -> Self {
pub fn new(
arrow_registry: &ArrowRegistry,
objects: &Objects,
obj: &Object,
hpp_type_extensions: &TokenStream,
) -> Self {
match obj.specifics {
crate::ObjectSpecifics::Struct => Self::from_struct(arrow_registry, objects, obj),
crate::ObjectSpecifics::Union { .. } => Self::from_union(arrow_registry, objects, obj),
crate::ObjectSpecifics::Struct => {
Self::from_struct(arrow_registry, objects, obj, hpp_type_extensions)
}
crate::ObjectSpecifics::Union { .. } => {
Self::from_union(arrow_registry, objects, obj, hpp_type_extensions)
}
}
}

fn from_struct(
arrow_registry: &ArrowRegistry,
objects: &Objects,
obj: &Object,
hpp_type_extensions: &TokenStream,
) -> QuotedObject {
let namespace_ident = format_ident!("{}", obj.kind.plural_snake_case()); // `datatypes`, `components`, or `archetypes`
let type_name = &obj.name;
Expand Down Expand Up @@ -254,14 +310,33 @@ impl QuotedObject {
// Single-field struct - it is a newtype wrapper.
// Create a implicit constructor and assignment from its own field-type.
let obj_field = &obj.fields[0];
if let Type::Array { .. } = &obj_field.typ {
// TODO(emilk): implicit constructor for arrays

let field_ident = format_ident!("{}", obj_field.name);
let param_ident = format_ident!("_{}", obj_field.name);

if let Type::Array { elem_type, length } = &obj_field.typ {
// Reminder: Arrays can't be passed by value, they decay to pointers. So we pass by reference!
// (we could pass by pointer, but if an extension wants to add smaller array constructor these would be ambiguous then!)
let length_quoted = quote_integer(length);
let element_type = quote_element_type(&mut hpp_includes, elem_type);
let element_assignments = (0..*length).map(|i| {
let i = quote_integer(i);
quote!(#param_ident[#i])
});
methods.push(Method {
declaration: MethodDeclaration::constructor(quote! {
#type_ident(const #element_type (&#param_ident)[#length_quoted]) : #field_ident{#(#element_assignments),*}
}),
..Method::default()
});

// No assignment operator for arrays since c arrays aren't typically assignable anyways.
// Note that creating an std::array overload would make initializer_list based construction ambiguous.
// Assignment operator for std::array could make sense though?
} else {
// Pass by value:
// If it was a temporary it gets moved into the value and then moved again into the field.
// If it was a lvalue it gets copied into the value and then moved into the field.
let field_ident = format_ident!("{}", obj_field.name);
let param_ident = format_ident!("_{}", obj_field.name);
let parameter_declaration =
quote_variable(&mut hpp_includes, obj_field, &param_ident);
hpp_includes.system.insert("utility".to_owned()); // std::move
Expand Down Expand Up @@ -412,7 +487,7 @@ impl QuotedObject {
},
},
definition_body: quote! {
#field_ident = std::move(std::vector(1, std::move(#parameter_ident)));
#field_ident = std::vector(1, std::move(#parameter_ident));
return *this;
},
inline: true,
Expand Down Expand Up @@ -476,6 +551,8 @@ impl QuotedObject {

#(#constants_hpp;)*

#hpp_type_extensions

#hpp_method_section
};
}
Expand All @@ -498,7 +575,12 @@ impl QuotedObject {
Self { hpp, cpp }
}

fn from_union(arrow_registry: &ArrowRegistry, objects: &Objects, obj: &Object) -> QuotedObject {
fn from_union(
arrow_registry: &ArrowRegistry,
objects: &Objects,
obj: &Object,
hpp_type_extensions: &TokenStream,
) -> QuotedObject {
// We implement sum-types as tagged unions;
// Putting non-POD types in a union requires C++11.
//
Expand Down Expand Up @@ -783,6 +865,8 @@ impl QuotedObject {

#destructor

#hpp_type_extensions

// This is useful for easily implementing the move constructor and assignment operators:
void swap(#pascal_case_ident& other) noexcept {
// Swap tags:
Expand Down
8 changes: 3 additions & 5 deletions docs/code-examples/arrow3d_simple_v2.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,10 @@ int main() {
for (int i = 0; i < 100; ++i) {
float angle = 2.0 * M_PI * i * 0.01f;
float length = log2f(i + 1);
vectors.push_back(rr::datatypes::Vec3D{length * sinf(angle), 0.0, length * cosf(angle)});
vectors.push_back({length * sinf(angle), 0.0, length * cosf(angle)});

// TODO(andreas): provide `unmultiplied_rgba`
uint8_t c = static_cast<uint8_t>((angle / (2.0 * M_PI) * 255.0) + 0.5);
uint32_t color = ((255 - c) << 24) + (c << 16) + (128 << 8) + (128 << 0);
colors.push_back(color);
uint8_t c = static_cast<uint8_t>(angle / (2.0 * M_PI) * 255.0);
colors.push_back({static_cast<uint8_t>(255 - c), c, 128, 128});
}

rr_stream.log("arrows", rr::archetypes::Arrows3D(vectors).with_colors(colors));
Expand Down
Loading

1 comment on commit 52d66ea

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Rust Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.25.

Benchmark suite Current: 52d66ea Previous: b63f875 Ratio
mono_points_arrow/generate_messages 225303516 ns/iter (± 2532257) 145430230 ns/iter (± 3579733) 1.55
mono_points_arrow/encode_total 517625935 ns/iter (± 39988507) 356440522 ns/iter (± 6265946) 1.45
mono_points_arrow/decode_message_bundles 94589509 ns/iter (± 3722958) 67391870 ns/iter (± 3107720) 1.40
mono_points_arrow/decode_total 416615979 ns/iter (± 63726057) 299030112 ns/iter (± 3265195) 1.39
arrow_mono_points/insert 2651404966 ns/iter (± 14044364) 1775246004 ns/iter (± 19527586) 1.49
arrow_mono_points2/insert 2730054662 ns/iter (± 35923271) 1776563900 ns/iter (± 13221213) 1.54

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.