Skip to content

Commit 9e50d2c

Browse files
emilkteh-cmc
andauthored
Ensure example .rrd files have description first (#7179)
### What * Closes #7175 ### 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 examples from latest `main` build: [rerun.io/viewer](https://rerun.io/viewer/pr/7179?manifest_url=https://app.rerun.io/version/main/examples_manifest.json) * Using full set of examples from `nightly` build: [rerun.io/viewer](https://rerun.io/viewer/pr/7179?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](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)! * [x] If have noted any breaking changes to the log API in `CHANGELOG.md` and the migration guide - [PR Build Summary](https://build.rerun.io/pr/7179) - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) To run all checks from `main`, comment on the PR with `@rerun-bot full-check`. --------- Co-authored-by: Clement Rey <cr.rey.clement@gmail.com>
1 parent 7108a90 commit 9e50d2c

File tree

9 files changed

+97
-92
lines changed

9 files changed

+97
-92
lines changed

Cargo.lock

+1
Original file line numberDiff line numberDiff line change
@@ -4486,6 +4486,7 @@ dependencies = [
44864486
"serde",
44874487
"serde_json",
44884488
"serde_yaml",
4489+
"tempfile",
44894490
"toml",
44904491
"ureq",
44914492
"url",

crates/build/re_dev_tools/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ rustdoc-types = "0.24.0"
3838
serde = { workspace = true, features = ["derive"] }
3939
serde_json.workspace = true
4040
serde_yaml.workspace = true
41+
tempfile.workspace = true
4142
toml = { workspace = true, features = ["parse", "preserve_order"] }
4243
ureq = { workspace = true, features = ["json"] }
4344
url.workspace = true

crates/build/re_dev_tools/src/build_examples/install.rs

+2-14
Original file line numberDiff line numberDiff line change
@@ -36,21 +36,9 @@ impl Install {
3636
}
3737

3838
let progress = MultiProgress::new();
39-
let output = wait_for_output(cmd, "installing examples", &progress)?;
39+
wait_for_output(cmd, "installing examples", &progress)?;
4040

41-
if output.status.success() {
42-
println!("Successfully installed examples");
43-
} else {
44-
anyhow::bail!(
45-
"Failed to install examples: \
46-
\nstdout: \
47-
\n{} \
48-
\nstderr: \
49-
\n{}",
50-
String::from_utf8(output.stdout)?,
51-
String::from_utf8(output.stderr)?,
52-
);
53-
}
41+
println!("Successfully installed examples");
5442

5543
Ok(())
5644
}

crates/build/re_dev_tools/src/build_examples/rrd.rs

+29-29
Original file line numberDiff line numberDiff line change
@@ -74,39 +74,39 @@ impl Rrd {
7474

7575
impl Example {
7676
fn build(self, progress: &MultiProgress, output_dir: &Path) -> anyhow::Result<PathBuf> {
77-
let rrd_path = output_dir.join(&self.name).with_extension("rrd");
77+
let tempdir = tempfile::tempdir()?;
7878

79-
let mut cmd = Command::new("python3");
80-
cmd.arg("-m").arg(&self.name);
81-
cmd.arg("--save").arg(&rrd_path);
82-
cmd.args(self.script_args);
83-
84-
let final_args = cmd
85-
.get_args()
86-
.map(|arg| arg.to_string_lossy().to_string())
87-
.collect::<Vec<_>>();
79+
let initial_rrd_path = tempdir.path().join(&self.name).with_extension("rrd");
8880

89-
// Configure flushing so that:
90-
// * the resulting file size is deterministic
91-
// * the file is chunked into small batches for better streaming
92-
cmd.env("RERUN_FLUSH_TICK_SECS", 1_000_000_000.to_string());
93-
cmd.env("RERUN_FLUSH_NUM_BYTES", (128 * 1024).to_string());
81+
{
82+
let mut cmd = Command::new("python3");
83+
cmd.arg("-m").arg(&self.name);
84+
cmd.arg("--save").arg(&initial_rrd_path);
85+
cmd.args(self.script_args);
9486

95-
let output = wait_for_output(cmd, &self.name, progress)?;
87+
// Configure flushing so that:
88+
// * the resulting file size is deterministic
89+
// * the file is chunked into small batches for better streaming
90+
cmd.env("RERUN_FLUSH_TICK_SECS", 1_000_000_000.to_string());
91+
cmd.env("RERUN_FLUSH_NUM_BYTES", (128 * 1024).to_string());
9692

97-
if output.status.success() {
98-
Ok(rrd_path)
99-
} else {
100-
anyhow::bail!(
101-
"Failed to run `python3 {}`: \
102-
\nstdout: \
103-
\n{} \
104-
\nstderr: \
105-
\n{}",
106-
final_args.join(" "),
107-
String::from_utf8(output.stdout)?,
108-
String::from_utf8(output.stderr)?,
109-
);
93+
wait_for_output(cmd, &self.name, progress)?;
11094
}
95+
96+
// Now run compaction on the result:
97+
let final_rrd_path = output_dir.join(&self.name).with_extension("rrd");
98+
99+
let mut cmd = Command::new("python3");
100+
cmd.arg("-m").arg("rerun");
101+
cmd.arg("rrd");
102+
cmd.arg("compact");
103+
// Small chunks for better streaming:
104+
cmd.arg("--max-bytes").arg((128 * 1024).to_string());
105+
cmd.arg(&initial_rrd_path);
106+
cmd.arg("-o").arg(&final_rrd_path);
107+
108+
wait_for_output(cmd, &format!("{} compaction", self.name), progress)?;
109+
110+
Ok(final_rrd_path)
111111
}
112112
}

crates/build/re_dev_tools/src/build_examples/snippets.rs

+3-21
Original file line numberDiff line numberDiff line change
@@ -160,11 +160,6 @@ impl Snippet {
160160
cmd.arg(&self.path);
161161
cmd.args(&self.extra_args);
162162

163-
let final_args = cmd
164-
.get_args()
165-
.map(|arg| arg.to_string_lossy().to_string())
166-
.collect::<Vec<_>>();
167-
168163
cmd.envs([
169164
("RERUN_FLUSH_NUM_ROWS", "0"),
170165
("RERUN_STRICT", "1"),
@@ -175,22 +170,9 @@ impl Snippet {
175170
),
176171
]);
177172

178-
let output = wait_for_output(cmd, &self.name, progress)?;
179-
180-
if output.status.success() {
181-
Ok(rrd_path)
182-
} else {
183-
anyhow::bail!(
184-
"Failed to run `python3 {}`: \
185-
\nstdout: \
186-
\n{} \
187-
\nstderr: \
188-
\n{}",
189-
final_args.join(" "),
190-
String::from_utf8(output.stdout)?,
191-
String::from_utf8(output.stderr)?,
192-
);
193-
}
173+
wait_for_output(cmd, &self.name, progress)?;
174+
175+
Ok(rrd_path)
194176
}
195177
}
196178

crates/build/re_dev_tools/src/build_examples/wait_for_output.rs

+23-3
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,24 @@
11
use std::io::stdout;
22
use std::io::IsTerminal;
33
use std::process::Command;
4-
use std::process::Output;
54
use std::time::Duration;
65

76
use indicatif::MultiProgress;
87
use indicatif::ProgressBar;
98

9+
/// Returns an error on non-zero returncode.
1010
pub fn wait_for_output(
1111
mut cmd: Command,
1212
name: &str,
1313
progress: &MultiProgress,
14-
) -> anyhow::Result<Output> {
14+
) -> anyhow::Result<()> {
15+
// Remember what we tried to run, for a better error message:
16+
let program = cmd.get_program().to_string_lossy().to_string();
17+
let args = cmd
18+
.get_args()
19+
.map(|arg| arg.to_string_lossy().to_string())
20+
.collect::<Vec<_>>();
21+
1522
let progress = progress.add(ProgressBar::new_spinner().with_message(name.to_owned()));
1623
progress.enable_steady_tick(Duration::from_millis(100));
1724

@@ -32,5 +39,18 @@ pub fn wait_for_output(
3239
println!("{message}");
3340
}
3441

35-
Ok(output)
42+
if !output.status.success() {
43+
let args = args.join(" ");
44+
let stdout = String::from_utf8(output.stdout)?;
45+
let stderr = String::from_utf8(output.stderr)?;
46+
anyhow::bail!(
47+
"Failed to run `{program} {args}`: \
48+
\nstdout: \
49+
\n{stdout} \
50+
\nstderr: \
51+
\n{stderr}",
52+
);
53+
}
54+
55+
Ok(())
3656
}

crates/store/re_chunk/src/chunk.rs

+3
Original file line numberDiff line numberDiff line change
@@ -833,6 +833,9 @@ impl Chunk {
833833
(times, counters)
834834
}
835835

836+
/// All the [`RowId`] in this chunk.
837+
///
838+
/// This could be in any order if this chunk is unsorted.
836839
#[inline]
837840
pub fn row_ids(&self) -> impl Iterator<Item = RowId> + '_ {
838841
let (times, counters) = self.row_ids_raw();

crates/store/re_chunk/src/transport.rs

+1
Original file line numberDiff line numberDiff line change
@@ -613,6 +613,7 @@ impl Chunk {
613613

614614
#[inline]
615615
pub fn to_arrow_msg(&self) -> ChunkResult<re_log_types::ArrowMsg> {
616+
re_tracing::profile_function!();
616617
self.sanity_check()?;
617618

618619
let transport = self.to_transport()?;

crates/store/re_entity_db/src/entity_db.rs

+34-25
Original file line numberDiff line numberDiff line change
@@ -466,35 +466,44 @@ impl EntityDb {
466466
.store_info_msg()
467467
.map(|msg| Ok(LogMsg::SetStoreInfo(msg.clone())));
468468

469-
let time_filter = time_selection.map(|(timeline, range)| {
470-
(
471-
timeline,
472-
ResolvedTimeRange::new(range.min.floor(), range.max.ceil()),
473-
)
474-
});
475-
476-
let data_messages = self
477-
.store()
478-
.iter_chunks()
479-
.filter(move |chunk| {
480-
let Some((timeline, time_range)) = time_filter else {
481-
return true;
482-
};
469+
let data_messages = {
470+
let time_filter = time_selection.map(|(timeline, range)| {
471+
(
472+
timeline,
473+
ResolvedTimeRange::new(range.min.floor(), range.max.ceil()),
474+
)
475+
});
483476

484-
// TODO(cmc): chunk.slice_time_selection(time_selection)
485-
chunk
486-
.timelines()
487-
.get(&timeline)
488-
.map_or(false, |time_column| {
489-
time_range.contains(time_column.time_range().min())
490-
|| time_range.contains(time_column.time_range().max())
491-
})
492-
})
493-
.map(|chunk| {
477+
let mut chunks: Vec<&Arc<Chunk>> = self
478+
.store()
479+
.iter_chunks()
480+
.filter(move |chunk| {
481+
let Some((timeline, time_range)) = time_filter else {
482+
return true;
483+
};
484+
485+
// TODO(cmc): chunk.slice_time_selection(time_selection)
486+
chunk
487+
.timelines()
488+
.get(&timeline)
489+
.map_or(false, |time_column| {
490+
time_range.contains(time_column.time_range().min())
491+
|| time_range.contains(time_column.time_range().max())
492+
})
493+
})
494+
.collect();
495+
496+
// Try to roughly preserve the order of the chunks
497+
// from how they were originally logged.
498+
// See https://github.com/rerun-io/rerun/issues/7175 for why.
499+
chunks.sort_by_key(|chunk| chunk.row_id_range().map(|(min, _)| min));
500+
501+
chunks.into_iter().map(|chunk| {
494502
chunk
495503
.to_arrow_msg()
496504
.map(|msg| LogMsg::ArrowMsg(self.store_id().clone(), msg))
497-
});
505+
})
506+
};
498507

499508
// If this is a blueprint, make sure to include the `BlueprintActivationCommand` message.
500509
// We generally use `to_messages` to export a blueprint via "save". In that

0 commit comments

Comments
 (0)