Skip to content

Commit

Permalink
A feature-flag to change maps representation to BTreeMap
Browse files Browse the repository at this point in the history
As of now, 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` feature flags which changes maps to be backed by
  [std::collections::BTreeMap](https://doc.rust-lang.org/std/collections/struct.BTreeMap.html).
  • Loading branch information
akhramov committed Dec 20, 2023
1 parent 4cffee8 commit 7717aa8
Show file tree
Hide file tree
Showing 12 changed files with 87 additions and 57 deletions.
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
11 changes: 6 additions & 5 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,8 +70,9 @@ 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<{}, {}>",
RustType::Map(ref key, ref value) => format!(
"{}::Map<{}, {}>",
protobuf_crate_path(customize),
key.to_code(customize),
value.to_code(customize)
),
Expand Down Expand Up @@ -220,7 +221,7 @@ 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(..) => format!("{}::Map::new()", protobuf_crate_path(customize)),
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 +267,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
2 changes: 2 additions & 0 deletions protobuf/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@ bench = false

[features]
with-bytes = ["bytes"]
btreemaps = []
default = []

[dependencies]
bytes = { version = "1.1", optional = true }
cfg-if = "1.0.0"
thiserror = "1.0.30"
once_cell = "1.9.0"

Expand Down
11 changes: 10 additions & 1 deletion protobuf/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@

## Features

This crate has one feature, which is `with-bytes`.
This crate has following features

### `with-bytes`

`with-bytes` enables `protobuf` crate support for
[`bytes` crate](https://github.com/tokio-rs/bytes):
Expand All @@ -17,6 +19,13 @@ just enabling option on this crate is not enough.

See `Customize` struct in [`protobuf-codegen` crate](https://docs.rs/protobuf-codegen).

### `btreemaps`
Modifies the Rust representation of
[map fields](https://protobuf.dev/programming-guides/proto3/#maps) to
use `BTreeMap` by default instead of `HashMap`. This can be handy for
those who want deterministic serialization. Note that unknown fields
still use hashmaps.

## Accompanying crates

* [`protobuf-json-mapping`](https://docs.rs/protobuf-json-mapping)
Expand Down
2 changes: 2 additions & 0 deletions protobuf/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ pub(crate) mod wire_format;
pub use crate::chars::Chars;
pub use crate::error::Error;
pub use crate::error::Result;
pub use crate::map::Map;

// generated
pub mod descriptor;
Expand Down Expand Up @@ -87,6 +88,7 @@ mod hex;
mod cached_size;
mod chars;
mod fixed;
mod map;
mod special;
mod unknown;
mod varint;
Expand Down
23 changes: 23 additions & 0 deletions protobuf/src/map.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
cfg_if::cfg_if! {
if #[cfg(feature = "btreemaps")] {
/// Rust type to map protobuf maps into.
pub type Map<K, V> = std::collections::BTreeMap<K, V>;

/// Key constraints for `Map`. BTreeMap keys must implement `Ord`
pub trait KeyConstraint: Ord {}
impl<T: Ord> KeyConstraint for T {}

/// Iterator type of `Map`
pub(crate) type Iter<'a, K, V> = std::collections::btree_map::Iter<'a, K, V>;
} else {
/// Rust type to map protobuf maps into.
pub type Map<K, V> = std::collections::HashMap<K, V>;

/// Key constraints for `Map`. HashMap keys must implement `std::hash::Hash`
pub trait KeyConstraint: std::hash::Hash {}
impl<T: std::hash::Hash> KeyConstraint for T {}

/// Iterator type of `Map`
pub(crate) type Iter<'a, K, V> = std::collections::hash_map::Iter<'a, K, V>;
}
}
21 changes: 10 additions & 11 deletions protobuf/src/reflect/acc/v2/map.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
use std::collections::HashMap;
use std::fmt;
use std::hash::Hash;

use crate::map::{self, Map};
use crate::message_dyn::MessageDyn;
use crate::message_full::MessageFull;
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 @@ -35,15 +34,15 @@ where
K: ProtobufValue,
V: ProtobufValue,
{
get_field: fn(&M) -> &HashMap<K, V>,
mut_field: fn(&mut M) -> &mut HashMap<K, V>,
get_field: fn(&M) -> &Map<K, V>,
mut_field: fn(&mut M) -> &mut Map<K, V>,
}

impl<M, K, V> MapFieldAccessor for MapFieldAccessorImpl<M, K, V>
where
M: MessageFull,
K: ProtobufValue + Eq + Hash,
K::RuntimeType: RuntimeTypeHashable,
K: ProtobufValue + Eq + map::KeyConstraint,
K::RuntimeType: RuntimeTypeMapKey,
V: ProtobufValue,
{
fn get_reflect<'a>(&self, m: &'a dyn MessageDyn) -> ReflectMapRef<'a> {
Expand All @@ -69,13 +68,13 @@ where
/// Make accessor for map field
pub fn make_map_simpler_accessor<M, K, V>(
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 Map<K, V>,
mut_field: for<'a> fn(&'a mut M) -> &'a mut Map<K, V>,
) -> FieldAccessor
where
M: MessageFull + 'static,
K: ProtobufValue + Hash + Eq,
K::RuntimeType: RuntimeTypeHashable,
K: ProtobufValue + map::KeyConstraint + Eq,
K::RuntimeType: RuntimeTypeMapKey,
V: ProtobufValue,
{
FieldAccessor::new(
Expand Down
23 changes: 10 additions & 13 deletions protobuf/src/reflect/map/generated.rs
Original file line number Diff line number Diff line change
@@ -1,37 +1,34 @@
use std::collections::hash_map;
use std::collections::HashMap;
use std::hash::Hash;

use crate::map::{self, Map};
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;
use crate::reflect::ReflectValueRef;
use crate::reflect::RuntimeType;

impl<K, V> ReflectMap for HashMap<K, V>
impl<K, V> ReflectMap for Map<K, V>
where
K: ProtobufValue + Eq + Hash,
K: ProtobufValue + Eq + map::KeyConstraint,
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() })
}

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

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

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>::map_get(self, key).map(V::RuntimeType::as_ref)
}

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

struct GeneratedMapIterImpl<'a, K: Eq + Hash + 'static, V: 'static> {
iter: hash_map::Iter<'a, K, V>,
struct GeneratedMapIterImpl<'a, K: Eq + map::KeyConstraint + 'static, V: 'static> {
iter: map::Iter<'a, K, V>,
}

impl<'a, K: ProtobufValue + Eq + Hash, V: ProtobufValue> ReflectMapIterTrait<'a>
impl<'a, K: ProtobufValue + Eq + map::KeyConstraint, V: ProtobufValue> ReflectMapIterTrait<'a>
for GeneratedMapIterImpl<'a, K, V>
{
fn next(&mut self) -> Option<(ReflectValueRef<'a>, ReflectValueRef<'a>)> {
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 `crate::Map` with appropriate keys and values
pub(crate) trait ReflectMap: Debug + Send + Sync + 'static {
fn reflect_iter(&self) -> ReflectMapIter;

Expand Down
37 changes: 18 additions & 19 deletions protobuf/src/reflect/runtime_types.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
//! Implementations of `RuntimeType` for all types.
use std::collections::HashMap;
use std::fmt;
use std::marker;

Expand All @@ -11,6 +10,7 @@ use bytes::Bytes;
use crate::chars::Chars;
use crate::descriptor::field_descriptor_proto::Type;
use crate::enum_or_unknown::EnumOrUnknown;
use crate::map::Map;
use crate::message_full::MessageFull;
use crate::reflect::runtime_type_box::RuntimeType;
use crate::reflect::types::ProtobufTypeBool;
Expand Down Expand Up @@ -118,11 +118,10 @@ pub trait RuntimeTypeWithDeref: RuntimeTypeTrait {
fn deref_as_ref(value: &Self::DerefTarget) -> ReflectValueRef;
}

/// Types which can be hashmap keys.
pub trait RuntimeTypeHashable: RuntimeTypeTrait {
/// Types which can be map keys.
pub trait RuntimeTypeMapKey: RuntimeTypeTrait {
/// Query hash map with a given key.
fn hash_map_get<'a, V>(map: &'a HashMap<Self::Value, V>, key: ReflectValueRef)
-> Option<&'a V>;
fn map_get<'a, V>(map: &'a Map<Self::Value, V>, key: ReflectValueRef) -> Option<&'a V>;
}

/// Implementation for `f32`
Expand Down Expand Up @@ -311,8 +310,8 @@ impl RuntimeTypeTrait for RuntimeTypeI32 {
}
}
}
impl RuntimeTypeHashable for RuntimeTypeI32 {
fn hash_map_get<'a, V>(map: &'a HashMap<i32, V>, key: ReflectValueRef) -> Option<&'a V> {
impl RuntimeTypeMapKey for RuntimeTypeI32 {
fn map_get<'a, V>(map: &'a Map<i32, V>, key: ReflectValueRef) -> Option<&'a V> {
match key {
ReflectValueRef::I32(i) => map.get(&i),
_ => None,
Expand Down Expand Up @@ -370,8 +369,8 @@ impl RuntimeTypeTrait for RuntimeTypeI64 {
}
}
}
impl RuntimeTypeHashable for RuntimeTypeI64 {
fn hash_map_get<'a, V>(map: &'a HashMap<i64, V>, key: ReflectValueRef) -> Option<&'a V> {
impl RuntimeTypeMapKey for RuntimeTypeI64 {
fn map_get<'a, V>(map: &'a Map<i64, V>, key: ReflectValueRef) -> Option<&'a V> {
match key {
ReflectValueRef::I64(i) => map.get(&i),
_ => None,
Expand Down Expand Up @@ -428,8 +427,8 @@ impl RuntimeTypeTrait for RuntimeTypeU32 {
}
}
}
impl RuntimeTypeHashable for RuntimeTypeU32 {
fn hash_map_get<'a, V>(map: &'a HashMap<u32, V>, key: ReflectValueRef) -> Option<&'a V> {
impl RuntimeTypeMapKey for RuntimeTypeU32 {
fn map_get<'a, V>(map: &'a Map<u32, V>, key: ReflectValueRef) -> Option<&'a V> {
match key {
ReflectValueRef::U32(i) => map.get(&i),
_ => None,
Expand Down Expand Up @@ -486,8 +485,8 @@ impl RuntimeTypeTrait for RuntimeTypeU64 {
}
}
}
impl RuntimeTypeHashable for RuntimeTypeU64 {
fn hash_map_get<'a, V>(map: &'a HashMap<u64, V>, key: ReflectValueRef) -> Option<&'a V> {
impl RuntimeTypeMapKey for RuntimeTypeU64 {
fn map_get<'a, V>(map: &'a Map<u64, V>, key: ReflectValueRef) -> Option<&'a V> {
match key {
ReflectValueRef::U64(i) => map.get(&i),
_ => None,
Expand Down Expand Up @@ -541,8 +540,8 @@ impl RuntimeTypeTrait for RuntimeTypeBool {
ProtobufTypeBool::get_from_unknown(unknown)
}
}
impl RuntimeTypeHashable for RuntimeTypeBool {
fn hash_map_get<'a, V>(map: &'a HashMap<bool, V>, key: ReflectValueRef) -> Option<&'a V> {
impl RuntimeTypeMapKey for RuntimeTypeBool {
fn map_get<'a, V>(map: &'a Map<bool, V>, key: ReflectValueRef) -> Option<&'a V> {
match key {
ReflectValueRef::Bool(i) => map.get(&i),
_ => None,
Expand Down Expand Up @@ -599,8 +598,8 @@ impl RuntimeTypeWithDeref for RuntimeTypeString {
ReflectValueRef::String(value)
}
}
impl RuntimeTypeHashable for RuntimeTypeString {
fn hash_map_get<'a, V>(map: &'a HashMap<String, V>, key: ReflectValueRef) -> Option<&'a V> {
impl RuntimeTypeMapKey for RuntimeTypeString {
fn map_get<'a, V>(map: &'a Map<String, V>, key: ReflectValueRef) -> Option<&'a V> {
match key {
ReflectValueRef::String(s) => map.get(*&s),
_ => None,
Expand Down Expand Up @@ -763,8 +762,8 @@ impl RuntimeTypeWithDeref for RuntimeTypeTokioChars {
}
}
#[cfg(feature = "bytes")]
impl RuntimeTypeHashable for RuntimeTypeTokioChars {
fn hash_map_get<'a, V>(map: &'a HashMap<Chars, V>, key: ReflectValueRef) -> Option<&'a V> {
impl RuntimeTypeMapKey for RuntimeTypeTokioChars {
fn map_get<'a, V>(map: &'a Map<Chars, V>, key: ReflectValueRef) -> Option<&'a V> {
match key {
ReflectValueRef::String(s) => map.get(&*s),
_ => None,
Expand Down
2 changes: 1 addition & 1 deletion protobuf/src/well_known_types/struct_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub struct Struct {
// message fields
/// Unordered map of dynamically typed values.
// @@protoc_insertion_point(field:google.protobuf.Struct.fields)
pub fields: ::std::collections::HashMap<::std::string::String, Value>,
pub fields: crate::Map<::std::string::String, Value>,
// special fields
// @@protoc_insertion_point(special_field:google.protobuf.Struct.special_fields)
pub special_fields: crate::SpecialFields,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
use std::collections::HashMap;

use protobuf::reflect::ReflectValueBox;
use protobuf::MessageFull;
use protobuf::{Map, MessageFull};

use super::test_reflect_clear_pb::*;

#[test]
fn test_generated() {
let mut map = HashMap::new();
let mut map = Map::new();
map.insert("key".to_string(), "value".to_string());

let mut msg = TestMessage::default();
Expand Down Expand Up @@ -45,7 +43,7 @@ fn test_dynamic() {

let mut msg = msg_desc.new_instance();

let mut map = HashMap::new();
let mut map = Map::new();
map.insert("key".to_string(), "value".to_string());

a_field.set_singular_field(msg.as_mut(), 1.into());
Expand Down

0 comments on commit 7717aa8

Please sign in to comment.