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

Codegen: option to use BTreeMap as map representation #696

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
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::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 @@
}
}

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 @@
}

/// 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`

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

View workflow job for this annotation

GitHub Actions / linux nightly (all features)

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 nightly (all features)

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 beta (default features)

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 beta (default features)

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 (default features)

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 (default features)

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
Loading