-
Notifications
You must be signed in to change notification settings - Fork 59
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
Support for other underlying smart pointers for the data structures links #7
Comments
We can't do But this is something I had in mind for a while, because of performance reasons. Why pay the price of having thread-safe reference counting when you do not need it (which is probably most of the time)? And there is a significant price to pay. For example, the So this is something that has a lot of value, however I really dislike the idea of solving this through code generation, specially since there are plans to support higher kinded types that would solve this nice and neatly. When rust supports them I will implement this. |
If you have a stomach for hacks it is sort of possible to emulate the HKT that are needed for this to work :). use std::rc::Rc;
use std::sync::Arc;
use std::ops::Deref;
trait Shared<T> {
type Ptr: Clone + Deref<Target = T>;
}
impl<T> Shared<T> for Rc<()> {
type Ptr = Rc<T>;
}
impl<T> Shared<T> for Arc<()> {
type Ptr = Arc<T>;
}
struct List<T, S> where S: Shared<T> {
field: S::Ptr,
}
struct Map<K, V, S> where S: Shared<K> + Shared<V> {
key: <S as Shared<K>>::Ptr,
value: <S as Shared<V>>::Ptr,
}
fn main() {
Map::<i32, &str, Rc<()>> {
key: Rc::new(1),
value: Rc::new(""),
};
} https://play.rust-lang.org/?gist=8b67f30a72e00dc3c2e3865bb31ae797&version=stable |
Hacked together #7 (comment) at https://github.com/Marwes/rpds/tree/rc . The speedups aren't very impressive so I am not sure if the added complexity is worth it.
|
Were seeing some odd performance problems when using incremental compilation where `Rc` pointers were actually slower than `Arc` pointers (the problem goes away when using non-incremental compilation). I haven't been able to build rustc locally to verify that this fixes it but these missing inline annotations seem to be the only thing that could affect performance (to this extent). ``` test vector_push_back ... bench: 11,668,015 ns/iter (+/- 772,861) test vector_push_back_mut ... bench: 1,423,771 ns/iter (+/- 22,011) test vector_push_back_mut_rc ... bench: 1,181,765 ns/iter (+/- 123,724) test vector_push_back_rc ... bench: 17,141,746 ns/iter (+/- 203,048) ``` (Source and non incremental benchmarks orium/rpds#7 (comment))
Add missing inline annotations to Cell Were seeing some odd performance problems when using incremental compilation where `Rc` pointers were actually slower than `Arc` pointers (the problem goes away when using non-incremental compilation). I haven't been able to build rustc locally to verify that this fixes it but these missing inline annotations seem to be the only thing that could affect performance (to this extent). ``` test vector_push_back ... bench: 11,668,015 ns/iter (+/- 772,861) test vector_push_back_mut ... bench: 1,423,771 ns/iter (+/- 22,011) test vector_push_back_mut_rc ... bench: 1,181,765 ns/iter (+/- 123,724) test vector_push_back_rc ... bench: 17,141,746 ns/iter (+/- 203,048) ``` (Source and non incremental benchmarks orium/rpds#7 (comment))
I measure a better speedup:
but even with this significant speedup I still don't think it is worth it, given the "hacky nature" of the solution. If everything goes well we should have generic associated types in rust by the end of this year (at least in nightly) and we will be able to have an elegant solution. |
I'd be (pleasantly) surprised if generic associated types appear this year though you may be right that this is a bit to hacky. That said, I fear that generic associated types will barely simplify anything. The main problem with this implementation is that The only actual benefit of generic associated types is that we don't have to specify bounds explicitly for every type that is behind a pointer https://github.com/Marwes/rpds/blob/e482d5abbaa6c876d7c624e497affe7299bbeece/src/sequence/vector/mod.rs#L153. |
Would top-level higher kinded types solve this? |
Maybe? This mostly depends on how the bounds in deriving gets specified. Something like below would be necessary. #[derive(Debug)]
struct List<T, P<_>> {
}
// Generates
impl<T, P> Debug for List<T, P<_>> where for<U> P<U>: Debug { } However that may actually be to broad of a bound in some case since we only need rust-lang/rfcs#2353 might help with this issue though. |
I guess |
Semi-related, it would be nice to omit the reference counted pointer for the values themselves and just store values as-is. For small types such as plain integers or floats as well as types which are already reference counted the |
Actually, if the keys and values are stored unboxed I think a less hacky solution may be used. I looked through a few of the structures (not all), and it seems that only a single type is boxed in an struct HashTrieMap<K, V, Entry> {
...
entry: Entry,
}
pub trait Shared: Deref {
fn new(t: Self::Target) -> Self;
fn make_mut(self_: &mut Self) -> &mut Self::Target;
}
impl<K, V, Entry> HashTrieMap<K, V, Entry> where Entry: Shared<Target = Entry<K, V>> { .. }
pub type ArcHashTrieMap<K, V> = HashTrieMap<K, V, Arc<Entry<K, V>>>;
pub type RcHashTrieMap<K, V> = HashTrieMap<K, V, Rc<Entry<K, V>>>; |
Splited this in two issues, one for to control the container of the elements of the data structure, which can be This one is now just for the links inside between the data structures nodes. |
Work in progress:
|
Nice! I was wondering when this would show up after seeing |
I may end up stealing the ideas in |
Awesome. Consider adding them to archery if they are general/flexible enough. In my mind I want archery to be more than just abstraction between |
Release 0.7.0 where you can choose between |
It would be nice to either use
Box
orRc
as the underlying smart pointer type. Alas, until we have associated generic types, we can't be polymorphic over this, but perhaps in the interim code generation could be used to create concrete implementations of:ArcList
RcList
BoxList
Then once associated generic types comes we could convert these to type aliases that refer to a common
List
type.The text was updated successfully, but these errors were encountered: