Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A feature-flag to change maps representation to BTreeMap #695

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
10 changes: 9 additions & 1 deletion protobuf/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +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):
when parsing bytes or strings from `bytes::Bytes`,
Expand All @@ -17,6 +18,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
12 changes: 11 additions & 1 deletion protobuf/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,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):
//! when parsing bytes or strings from `bytes::Bytes`,
Expand All @@ -15,6 +16,13 @@
//!
//! 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 Expand Up @@ -51,6 +59,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 +96,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
Loading
Loading