Skip to content

Commit

Permalink
Codegen: option to use BTreeMap as map representation
Browse files Browse the repository at this point in the history
As of now, protobuf maps are represented with
[std::collections::HashMap](https://doc.rust-lang.org/stable/std/collections/struct.HashMap.html),
which serves as a robust default. In rarer instances, opting for a
different implementation, such as BTreeMap, might be desirable,
e.g. for deterministic serialization (#496).

This change

* adds `btreemaps` codegen option to generate
  [std::collections::BTreeMap](https://doc.rust-lang.org/std/collections/struct.BTreeMap.html)
  for maps representation.
  • Loading branch information
akhramov committed Dec 22, 2023
1 parent 4cffee8 commit cfa0f0c
Show file tree
Hide file tree
Showing 19 changed files with 429 additions and 67 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ members = [
"test-crates/perftest/bytes",
"test-crates/perftest/misc",
"test-crates/perftest/vs-cxx",
"test-crates/protobuf-codegen-btreemap-test",
"test-crates/protobuf-codegen-identical-test",
"test-crates/protobuf-codegen-protoc-test",
"test-crates/protobuf-codegen-pure-test",
Expand Down
13 changes: 13 additions & 0 deletions protobuf-codegen/src/customize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ pub struct Customize {
/// Used internally to generate protos bundled in protobuf crate
/// like `descriptor.proto`
pub(crate) inside_protobuf: Option<bool>,
/// When true, protobuf maps are represented with `std::collections::BTreeMap`
pub(crate) btreemaps: Option<bool>,
}

#[derive(Debug, thiserror::Error)]
Expand Down Expand Up @@ -171,6 +173,14 @@ impl Customize {
self
}

/// Use btreemaps for maps representation
pub fn btreemaps(self, use_btreemaps: bool) -> Self {
Self {
btreemaps: Some(use_btreemaps),
..self
}
}

/// Update fields of self with fields defined in other customize
pub fn update_with(&mut self, that: &Customize) {
if let Some(v) = &that.before {
Expand All @@ -197,6 +207,9 @@ impl Customize {
if let Some(v) = that.inside_protobuf {
self.inside_protobuf = Some(v);
}
if let Some(v) = that.btreemaps {
self.btreemaps = Some(v);
}
}

/// Update unset fields of self with fields from other customize
Expand Down
6 changes: 6 additions & 0 deletions protobuf-codegen/src/customize/rustproto_proto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ pub(crate) fn customize_from_rustproto_for_message(source: &MessageOptions) -> C
let lite_runtime = None;
let gen_mod_rs = None;
let inside_protobuf = None;
let btreemaps = None;
Customize {
before,
generate_accessors,
Expand All @@ -24,6 +25,7 @@ pub(crate) fn customize_from_rustproto_for_message(source: &MessageOptions) -> C
lite_runtime,
gen_mod_rs,
inside_protobuf,
btreemaps,
}
}

Expand All @@ -40,6 +42,7 @@ pub(crate) fn customize_from_rustproto_for_field(source: &FieldOptions) -> Custo
let lite_runtime = None;
let gen_mod_rs = None;
let inside_protobuf = None;
let btreemaps = None;
Customize {
before,
generate_accessors,
Expand All @@ -49,6 +52,7 @@ pub(crate) fn customize_from_rustproto_for_field(source: &FieldOptions) -> Custo
lite_runtime,
gen_mod_rs,
inside_protobuf,
btreemaps,
}
}

Expand All @@ -61,6 +65,7 @@ pub(crate) fn customize_from_rustproto_for_file(source: &FileOptions) -> Customi
let lite_runtime = rustproto::exts::lite_runtime_all.get(source);
let gen_mod_rs = None;
let inside_protobuf = None;
let btreemaps = None;
Customize {
before,
generate_accessors,
Expand All @@ -70,5 +75,6 @@ pub(crate) fn customize_from_rustproto_for_file(source: &FileOptions) -> Customi
lite_runtime,
inside_protobuf,
gen_mod_rs,
btreemaps,
}
}
2 changes: 1 addition & 1 deletion protobuf-codegen/src/gen/field/accessor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ impl FieldGen<'_> {
let MapField { .. } = map_field;
AccessorFn {
name: "make_map_simpler_accessor".to_owned(),
type_params: vec![format!("_"), format!("_")],
type_params: vec![format!("_")],
callback_params: self.make_accessor_fns_lambda(),
}
}
Expand Down
2 changes: 1 addition & 1 deletion protobuf-codegen/src/gen/field/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ impl<'a> FieldGen<'a> {
FieldKind::Repeated(ref repeated) => repeated.rust_type(reference),
FieldKind::Map(MapField {
ref key, ref value, ..
}) => RustType::HashMap(
}) => RustType::Map(
Box::new(key.rust_storage_elem_type(reference)),
Box::new(value.rust_storage_elem_type(reference)),
),
Expand Down
31 changes: 23 additions & 8 deletions protobuf-codegen/src/gen/rust_types_values.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ pub(crate) enum RustType {
Float(u32),
Bool,
Vec(Box<RustType>),
HashMap(Box<RustType>, Box<RustType>),
Map(Box<RustType>, Box<RustType>),
String,
// [T], not &[T]
Slice(Box<RustType>),
Expand Down Expand Up @@ -70,11 +70,19 @@ impl RustType {
RustType::Float(bits) => format!("f{}", bits),
RustType::Bool => format!("bool"),
RustType::Vec(ref param) => format!("::std::vec::Vec<{}>", param.to_code(customize)),
RustType::HashMap(ref key, ref value) => format!(
"::std::collections::HashMap<{}, {}>",
key.to_code(customize),
value.to_code(customize)
),
RustType::Map(ref key, ref value) => {
let ty = if let Some(true) = customize.btreemaps {
"::std::collections::BTreeMap"
} else {
"::std::collections::HashMap"
};
format!(
"{}<{}, {}>",
ty,
key.to_code(customize),
value.to_code(customize)
)
}
RustType::String => format!("::std::string::String"),
RustType::Slice(ref param) => format!("[{}]", param.to_code(customize)),
RustType::Str => format!("str"),
Expand Down Expand Up @@ -220,7 +228,14 @@ impl RustType {
RustType::Float(..) => "0.".to_string(),
RustType::Bool => "false".to_string(),
RustType::Vec(..) => EXPR_VEC_NEW.to_string(),
RustType::HashMap(..) => "::std::collections::HashMap::new()".to_string(),
RustType::Map(..) => {
let ty = if let Some(true) = customize.btreemaps {
"::std::collections::BTreeMap"
} else {
"::std::collections::HashMap"
};
format!("{}::new()", ty)
}
RustType::String => "::std::string::String::new()".to_string(),
RustType::Bytes => "::bytes::Bytes::new()".to_string(),
RustType::Chars => format!("{}::Chars::new()", protobuf_crate_path(customize)),
Expand Down Expand Up @@ -266,7 +281,7 @@ impl RustType {
| RustType::Chars
| RustType::String
| RustType::MessageField(..)
| RustType::HashMap(..) => format!("{}.clear()", v),
| RustType::Map(..) => format!("{}.clear()", v),
RustType::Bool
| RustType::Float(..)
| RustType::Int(..)
Expand Down
57 changes: 40 additions & 17 deletions protobuf/src/reflect/acc/v2/map.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::collections::HashMap;
use std::collections::{BTreeMap, HashMap};
use std::fmt;
use std::hash::Hash;

Expand All @@ -8,7 +8,7 @@ use crate::reflect::acc::v2::AccessorV2;
use crate::reflect::acc::FieldAccessor;
use crate::reflect::map::ReflectMapMut;
use crate::reflect::map::ReflectMapRef;
use crate::reflect::runtime_types::RuntimeTypeHashable;
use crate::reflect::runtime_types::RuntimeTypeMapKey;
use crate::reflect::runtime_types::RuntimeTypeTrait;
use crate::reflect::ProtobufValue;
use crate::reflect::RuntimeType;
Expand All @@ -29,21 +29,46 @@ impl<'a> fmt::Debug for MapFieldAccessorHolder {
}
}

struct MapFieldAccessorImpl<M, K, V>
struct MapFieldAccessorImpl<M, T>
where
M: MessageFull,
K: ProtobufValue,
V: ProtobufValue,
{
get_field: fn(&M) -> &HashMap<K, V>,
mut_field: fn(&mut M) -> &mut HashMap<K, V>,
get_field: fn(&M) -> &T,
mut_field: fn(&mut M) -> &mut T,
}

impl<M, K, V> MapFieldAccessor for MapFieldAccessorImpl<M, K, V>
impl<M, K, V> MapFieldAccessor for MapFieldAccessorImpl<M, HashMap<K, V>>
where
M: MessageFull,
K: ProtobufValue + Eq + Hash,
K::RuntimeType: RuntimeTypeHashable,
K::RuntimeType: RuntimeTypeMapKey,
V: ProtobufValue,
{
fn get_reflect<'a>(&self, m: &'a dyn MessageDyn) -> ReflectMapRef<'a> {
let m = m.downcast_ref().unwrap();
let map = (self.get_field)(m);
ReflectMapRef::new(map)
}

fn mut_reflect<'a>(&self, m: &'a mut dyn MessageDyn) -> ReflectMapMut<'a> {
let m = m.downcast_mut().unwrap();
let map = (self.mut_field)(m);
ReflectMapMut::new(map)
}

fn element_type(&self) -> (RuntimeType, RuntimeType) {
(
K::RuntimeType::runtime_type_box(),
V::RuntimeType::runtime_type_box(),
)
}
}

impl<M, K, V> MapFieldAccessor for MapFieldAccessorImpl<M, BTreeMap<K, V>>
where
M: MessageFull,
K: ProtobufValue + Ord,
K::RuntimeType: RuntimeTypeMapKey,
V: ProtobufValue,
{
fn get_reflect<'a>(&self, m: &'a dyn MessageDyn) -> ReflectMapRef<'a> {
Expand All @@ -67,21 +92,19 @@ where
}

/// Make accessor for map field
pub fn make_map_simpler_accessor<M, K, V>(
pub fn make_map_simpler_accessor<M, T>(

Check warning on line 95 in protobuf/src/reflect/acc/v2/map.rs

View workflow job for this annotation

GitHub Actions / rustfmt check

trait `MapFieldAccessor` is more private than the item `make_map_simpler_accessor`

Check warning on line 95 in protobuf/src/reflect/acc/v2/map.rs

View workflow job for this annotation

GitHub Actions / rustfmt check

type `MapFieldAccessorImpl<M, T>` is more private than the item `make_map_simpler_accessor`

Check warning on line 95 in protobuf/src/reflect/acc/v2/map.rs

View workflow job for this annotation

GitHub Actions / Miri test

trait `reflect::acc::v2::map::MapFieldAccessor` is more private than the item `reflect::acc::v2::map::make_map_simpler_accessor`

Check warning on line 95 in protobuf/src/reflect/acc/v2/map.rs

View workflow job for this annotation

GitHub Actions / Miri test

type `reflect::acc::v2::map::MapFieldAccessorImpl<M, T>` is more private than the item `reflect::acc::v2::map::make_map_simpler_accessor`

Check warning on line 95 in protobuf/src/reflect/acc/v2/map.rs

View workflow job for this annotation

GitHub Actions / linux stable (with-bytes)

trait `MapFieldAccessor` is more private than the item `make_map_simpler_accessor`

Check warning on line 95 in protobuf/src/reflect/acc/v2/map.rs

View workflow job for this annotation

GitHub Actions / linux stable (with-bytes)

type `MapFieldAccessorImpl<M, T>` is more private than the item `make_map_simpler_accessor`

Check warning on line 95 in protobuf/src/reflect/acc/v2/map.rs

View workflow job for this annotation

GitHub Actions / linux stable (with-bytes)

trait `MapFieldAccessor` is more private than the item `make_map_simpler_accessor`

Check warning on line 95 in protobuf/src/reflect/acc/v2/map.rs

View workflow job for this annotation

GitHub Actions / linux stable (with-bytes)

type `MapFieldAccessorImpl<M, T>` is more private than the item `make_map_simpler_accessor`
name: &'static str,
get_field: for<'a> fn(&'a M) -> &'a HashMap<K, V>,
mut_field: for<'a> fn(&'a mut M) -> &'a mut HashMap<K, V>,
get_field: for<'a> fn(&'a M) -> &'a T,
mut_field: for<'a> fn(&'a mut M) -> &'a mut T,
) -> FieldAccessor
where
M: MessageFull + 'static,
K: ProtobufValue + Hash + Eq,
K::RuntimeType: RuntimeTypeHashable,
V: ProtobufValue,
M: MessageFull,
MapFieldAccessorImpl<M, T>: MapFieldAccessor,
{
FieldAccessor::new(
name,
AccessorV2::Map(MapFieldAccessorHolder {
accessor: Box::new(MapFieldAccessorImpl::<M, K, V> {
accessor: Box::new(MapFieldAccessorImpl::<M, T> {
get_field,
mut_field,
}),
Expand Down
65 changes: 55 additions & 10 deletions protobuf/src/reflect/map/generated.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use std::collections::hash_map;
use std::collections::HashMap;
use std::collections::{btree_map, hash_map};
use std::collections::{BTreeMap, HashMap};
use std::hash::Hash;

use crate::reflect::map::ReflectMap;
use crate::reflect::map::ReflectMapIter;
use crate::reflect::map::ReflectMapIterTrait;
use crate::reflect::runtime_types::RuntimeTypeHashable;
use crate::reflect::runtime_types::RuntimeTypeMapKey;
use crate::reflect::runtime_types::RuntimeTypeTrait;
use crate::reflect::ProtobufValue;
use crate::reflect::ReflectValueBox;
Expand All @@ -16,10 +16,12 @@ impl<K, V> ReflectMap for HashMap<K, V>
where
K: ProtobufValue + Eq + Hash,
V: ProtobufValue,
K::RuntimeType: RuntimeTypeHashable,
K::RuntimeType: RuntimeTypeMapKey,
{
fn reflect_iter<'a>(&'a self) -> ReflectMapIter<'a> {
ReflectMapIter::new(GeneratedMapIterImpl::<'a, K, V> { iter: self.iter() })
ReflectMapIter::new(GeneratedMapIterImpl::<'a, K, V, hash_map::Iter<'a, K, V>> {
iter: self.iter(),
})
}

fn len(&self) -> usize {
Expand All @@ -31,7 +33,7 @@ where
}

fn get<'a>(&'a self, key: ReflectValueRef) -> Option<ReflectValueRef<'a>> {
<K::RuntimeType as RuntimeTypeHashable>::hash_map_get(self, key).map(V::RuntimeType::as_ref)
<K::RuntimeType as RuntimeTypeMapKey>::hash_map_get(self, key).map(V::RuntimeType::as_ref)
}

fn insert(&mut self, key: ReflectValueBox, value: ReflectValueBox) {
Expand All @@ -53,12 +55,55 @@ where
}
}

struct GeneratedMapIterImpl<'a, K: Eq + Hash + 'static, V: 'static> {
iter: hash_map::Iter<'a, K, V>,
impl<K, V> ReflectMap for BTreeMap<K, V>
where
K: ProtobufValue + Ord,
V: ProtobufValue,
K::RuntimeType: RuntimeTypeMapKey,
{
fn reflect_iter<'a>(&'a self) -> ReflectMapIter<'a> {
ReflectMapIter::new(
GeneratedMapIterImpl::<'a, K, V, btree_map::Iter<'a, K, V>> { iter: self.iter() },
)
}

fn len(&self) -> usize {
BTreeMap::len(self)
}

fn is_empty(&self) -> bool {
self.is_empty()
}

fn get<'a>(&'a self, key: ReflectValueRef) -> Option<ReflectValueRef<'a>> {
<K::RuntimeType as RuntimeTypeMapKey>::btree_map_get(self, key).map(V::RuntimeType::as_ref)
}

fn insert(&mut self, key: ReflectValueBox, value: ReflectValueBox) {
let key: K = key.downcast().expect("wrong key type");
let value: V = value.downcast().expect("wrong value type");
self.insert(key, value);
}

fn clear(&mut self) {
self.clear();
}

fn key_type(&self) -> RuntimeType {
K::RuntimeType::runtime_type_box()
}

fn value_type(&self) -> RuntimeType {
V::RuntimeType::runtime_type_box()
}
}

struct GeneratedMapIterImpl<'a, K: 'static, V: 'static, I: Iterator<Item = (&'a K, &'a V)>> {
iter: I,
}

impl<'a, K: ProtobufValue + Eq + Hash, V: ProtobufValue> ReflectMapIterTrait<'a>
for GeneratedMapIterImpl<'a, K, V>
impl<'a, K: ProtobufValue, V: ProtobufValue, I: Iterator<Item = (&'a K, &'a V)>>
ReflectMapIterTrait<'a> for GeneratedMapIterImpl<'a, K, V, I>
{
fn next(&mut self) -> Option<(ReflectValueRef<'a>, ReflectValueRef<'a>)> {
match self.iter.next() {
Expand Down
2 changes: 1 addition & 1 deletion protobuf/src/reflect/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::reflect::RuntimeType;
mod empty;
mod generated;

/// Implemented for `HashMap` with appropriate keys and values
/// Implemented for `HashMap`, `BTreeMap` with appropriate keys and values
pub(crate) trait ReflectMap: Debug + Send + Sync + 'static {
fn reflect_iter(&self) -> ReflectMapIter;

Expand Down
Loading

0 comments on commit cfa0f0c

Please sign in to comment.