Skip to content

Commit

Permalink
Add benches, implement new constructor APIs with docs, clippy
Browse files Browse the repository at this point in the history
  • Loading branch information
sffc committed Nov 2, 2023
1 parent e28d655 commit 4399055
Show file tree
Hide file tree
Showing 7 changed files with 100 additions and 34 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

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

5 changes: 5 additions & 0 deletions provider/blob/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ log = { version = "0.4", optional = true }
[dev-dependencies]
icu_locid = { path = "../../components/locid", features = ["serde"] }
icu_datagen = { path = "../../provider/datagen", features = ["networking"] }
criterion = "0.4"

[features]
std = ["icu_provider/std"]
Expand All @@ -44,3 +45,7 @@ export = [
"zerotrie/alloc",
"zerotrie/litemap",
]

[[bench]]
name = "blob_version_bench"
harness = false
58 changes: 58 additions & 0 deletions provider/blob/benches/blob_version_bench.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// This file is part of ICU4X. For terms of use, please see the file
// called LICENSE at the top level of the ICU4X source tree
// (online at: https://github.com/unicode-org/icu4x/blob/main/LICENSE ).

extern crate alloc;

use criterion::{black_box, criterion_group, criterion_main, Criterion};
use icu_provider::datagen::IterableDataProvider;
use icu_provider::hello_world::*;
use icu_provider::prelude::*;
use icu_provider_blob::BlobDataProvider;

const BLOB_V1: &[u8] = include_bytes!("../tests/data/v1.postcard");
const BLOB_V2: &[u8] = include_bytes!("../tests/data/v2.postcard");

fn blob_version_bench(c: &mut Criterion) {
c.bench_function("provider/construct/v1", |b| {
b.iter(|| BlobDataProvider::try_new_from_static_blob(black_box(BLOB_V1)).unwrap());
});
c.bench_function("provider/construct/v2", |b| {
b.iter(|| BlobDataProvider::try_new_from_static_blob(black_box(BLOB_V1)).unwrap());
});

let hello_world_provider = HelloWorldProvider;
let locales = hello_world_provider.supported_locales().unwrap();

c.bench_function("provider/read/v1", |b| {
let provider = BlobDataProvider::try_new_from_static_blob(black_box(BLOB_V1)).unwrap();
b.iter(|| {
for locale in black_box(&locales).iter() {
let _: DataResponse<HelloWorldV1Marker> = black_box(&provider)
.as_deserializing()
.load(DataRequest {
locale,
metadata: Default::default(),
})
.unwrap();
}
});
});
c.bench_function("provider/read/v2", |b| {
let provider = BlobDataProvider::try_new_from_static_blob(black_box(BLOB_V2)).unwrap();
b.iter(|| {
for locale in black_box(&locales).iter() {
let _: DataResponse<HelloWorldV1Marker> = black_box(&provider)
.as_deserializing()
.load(DataRequest {
locale,
metadata: Default::default(),
})
.unwrap();
}
});
});
}

criterion_group!(benches, blob_version_bench,);
criterion_main!(benches);
20 changes: 8 additions & 12 deletions provider/blob/src/blob_data_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,13 +156,11 @@ mod test {
let mut blob: Vec<u8> = Vec::new();

{
let mut exporter = BlobExporter::new_with_sink(Box::new(&mut blob));

if version == 1 {
exporter.set_v1();
let mut exporter = if version == 1 {
BlobExporter::new_with_sink(Box::new(&mut blob))
} else {
exporter.set_v2();
}
BlobExporter::new_v2_with_sink(Box::new(&mut blob))
};

exporter.flush(HelloWorldV1Marker::KEY).unwrap();

Expand Down Expand Up @@ -194,13 +192,11 @@ mod test {
let mut blob: Vec<u8> = Vec::new();

{
let mut exporter = BlobExporter::new_with_sink(Box::new(&mut blob));

if version == 1 {
exporter.set_v1();
let mut exporter = if version == 1 {
BlobExporter::new_with_sink(Box::new(&mut blob))
} else {
exporter.set_v2();
}
BlobExporter::new_v2_with_sink(Box::new(&mut blob))
};

exporter.flush(HelloSingletonV1Marker::KEY).unwrap();

Expand Down
32 changes: 20 additions & 12 deletions provider/blob/src/export/blob_exporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,10 @@ impl core::fmt::Debug for BlobExporter<'_> {
}

impl<'w> BlobExporter<'w> {
/// Create a [`BlobExporter`] that writes to the given I/O stream.
/// Creates a version 1 [`BlobExporter`] that writes to the given I/O stream.
///
/// Version 1 is needed if the blob may be consumed by ICU4X versions 1.0 through 1.3. If
/// targeting only ICU4X 1.4 and above, see [BlobExporter::new_v2_with_sink()].
pub fn new_with_sink(sink: Box<dyn std::io::Write + Sync + 'w>) -> Self {
Self {
resources: Mutex::new(BTreeMap::new()),
Expand All @@ -62,14 +65,19 @@ impl<'w> BlobExporter<'w> {
}
}

/// Set the exporter to produce v1 blobs.
pub fn set_v1(&mut self) {
self.version = VersionConfig::V001;
}

/// Set the exporter to produce v2 blobs.
pub fn set_v2(&mut self) {
self.version = VersionConfig::V002;
/// Creates a version 2 [`BlobExporter`] that writes to the given I/O stream.
///
/// Version 2 produces a smaller postcard file than version 1 without sacrificing performance.
/// It is compatible with ICU4X 1.4 and above. If you need to support older version of ICU4X,
/// see [BlobExporter::new_with_sink()].
pub fn new_v2_with_sink(sink: Box<dyn std::io::Write + Sync + 'w>) -> Self {
Self {
resources: Mutex::new(BTreeMap::new()),
unique_resources: Mutex::new(HashMap::new()),
all_keys: Mutex::new(BTreeSet::new()),
sink,
version: VersionConfig::V002,
}
}
}

Expand Down Expand Up @@ -98,7 +106,7 @@ impl DataExporter for BlobExporter<'_> {
.lock()
.expect("poison")
.entry(key.hashed())
.or_insert_with(Default::default)
.or_default()
.entry(locale.write_to_string().into_owned().into_bytes())
.or_insert(idx);
Ok(())
Expand Down Expand Up @@ -226,8 +234,8 @@ impl BlobExporter<'_> {

if !keys.is_empty() {
let blob = BlobSchema::V002(BlobSchemaV2 {
keys: &*keys,
locales: &*locales_vzv,
keys: &keys,
locales: &locales_vzv,
buffers: &vzv,
});
log::info!("Serializing blob to output stream...");
Expand Down
5 changes: 2 additions & 3 deletions provider/blob/tests/test_versions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ fn run_driver(exporter: BlobExporter) -> Result<(), DataError> {
}

fn check_hello_world(blob_provider: impl DataProvider<HelloWorldV1Marker>) {
let hello_world_provider = HelloWorldProvider::default();
let hello_world_provider = HelloWorldProvider;
for locale in hello_world_provider.supported_locales().unwrap() {
let blob_result = blob_provider
.load(DataRequest {
Expand Down Expand Up @@ -56,8 +56,7 @@ fn test_v1() {
#[test]
fn test_v2() {
let mut blob: Vec<u8> = Vec::new();
let mut exporter = BlobExporter::new_with_sink(Box::new(&mut blob));
exporter.set_v2();
let exporter = BlobExporter::new_v2_with_sink(Box::new(&mut blob));
run_driver(exporter).unwrap();
assert_eq!(BLOB_V2, blob.as_slice());

Expand Down
13 changes: 6 additions & 7 deletions provider/datagen/src/bin/datagen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ fn main() -> eyre::Result<()> {
eyre::bail!("Exporting to a BlobProvider requires the `blob_exporter` Cargo feature");
#[cfg(feature = "blob_exporter")]
{
let mut exporter = icu_provider_blob::export::BlobExporter::new_with_sink(
let sink: Box<dyn std::io::Write + Sync> =
if path == std::path::Path::new("/stdout") {
Box::new(std::io::stdout())
} else if !config.overwrite && path.exists() {
Expand All @@ -168,13 +168,12 @@ fn main() -> eyre::Result<()> {
std::fs::File::create(path)
.with_context(|| path.to_string_lossy().to_string())?,
)
},
);
if matches!(config.export, config::Export::Blob { .. }) {
exporter.set_v1();
};
let exporter = if matches!(config.export, config::Export::Blob { .. }) {
icu_provider_blob::export::BlobExporter::new_with_sink(sink)
} else {
exporter.set_v2();
}
icu_provider_blob::export::BlobExporter::new_v2_with_sink(sink)
};
Ok(driver.export(&provider, exporter)?)
}
}
Expand Down

0 comments on commit 4399055

Please sign in to comment.