Zitga-Tech
Some suggestions
I have not read the code fully,but here is my suggestion:
- I think the ArrayDictionary should be named SortedList.It should be a big mistake if the ArrayDictionary has same logic with SortedList. 2.What is different between ArrayHashSet to HashSet?ArrayHashSet<T> has same logic with SortedList<T,Unit>?If they are same,it is better that remove the ArrayHashSet<T> from the library. 3.Why not use RefXXX name instead of TempXXX ? 4.I find no warnning about TempXXX. Stack space is limited and will cause Exception while alloc too much.
I have my repo PooledBuffer base on jtmueller/Collections.Pooled,but I found a problem with jtmueller/Collections.Pooled a few day ago and it looks like dead.I am finding a new library to replace it ,and I get this library.But your library seems not ready and not strong enough.You can do this better.
Hi, thank you for paying attention to this library. Let me answer your questions then I will tell you the state of this lib.
1+2: Truthfully, at the time of making this library I had no knowledge about SortedList. But a quick search tells me that the 2 are different things. In fact, ArrayDictionary is an older version of SveltoDictionary, which is described as:
This dictionary has been created for just one reason: I needed a dictionary that would have let me iterate over the values as an array, directly, without generating one or using an iterator. For this goal is N times faster than the standard dictionary. Faster dictionary is also faster than the standard dictionary for most of the operations, but the difference is negligible. The only slower operation is resizing the memory on add, as this implementation needs to use two separate arrays compared to the standard one SveltoDictionary.cs#L45
As I understand, SortedList has its keys sorted. On the other hand, ArrayDictionary doesn't claim to have this behavior. This is also true with ArrayHashSet, which is just a stripped-down version of ArrayDictionary.
3: I can only say naming is a thing to debate. I named them Temp- because I think they should be treated as temporary container.
4: I'm not sure I understand what you mean. Temp- containers don't use stack alloc. Behind the name, it's just normal heap alloc utilizing ArrayPool. This library is not designed to use with stackalloc.
Lastly, this library was developed at the time my studio was researching about optimizations across many aspects of a Unity project. The truth is, today its usage is very limited inside my studio. That's why it hasn't received any update for a while. Currently, while working on newer projects, we will try to use Unity Collections as much as we can, and the needs for poolable managed collections are lesser than before.
The above reasons, coupled with the fact that we're quite busy with the game projects, we can't commit to update this library. However, you're free to make PRs, or fork this repo for your own use as it's distributed with MIT license.
Note: The source code of this library is mostly copied from .NET Generic Collections and Svelto Dictionary with some changes on how internal arrays are governed utilized ArrayPool. You can update them with the up-to-date version from those upstream repos. There are some methods you have to extract from the .NET codebase for they are designed to be reusable but only internally.
Building a robust foundational data structure takes a lot of brainpower and time. Currently, I don't have the energy or time to spare for improving the codebase (which is why I'm looking for alternatives to Collection.Pool rather than forking it directly), so I won't be submitting a PR (at least not in the short term). You can treat the questions, doubts, and suggestions raised here purely as academic discussion.
Certainly, I still haven't had the time to thoroughly read through your library's code, nor have I read the code of SveltoDictionary. But based on your responses, I'd like to express the following viewpoints:
If SveltoDictionary.cs#L45 functions as described, its functionality is entirely equivalent to SortedList. I'm not familiar with the actual algorithmic differences between the two, but SortedList is well-written and performs well; it's based on BinarySearch.
Naming is an art, and the Temp- prefix isn't an issue. The problem lies in the fact that your data structure appears in the form of a ref struct, which led me to believe it was a stack-based data structure. Hence, I suggested using a Ref prefix. However, since it's not stack-based, it seems unnecessary to separately implement a ref series of collections apart from the Value series. If the Value series collections have the same logic, purpose, and performance as the ref series, then the existence of the ref series could become a pitfall in the future.
I didn't mention this earlier, but while trying out your library, I encountered a serious bug. I believe it may have been inherited from the upstream author when referencing Collection.Pool (which also has this bug) into their own codebase. After clearing the dictionary and then assigning values to it using the indexer, it leads to garbage collection. If widely used in Unity, this can cause more frequent garbage collection, so it should be taken seriously.
After clearing the dictionary and then assigning values to it using the indexer, it leads to garbage collection.
Thanks for reporting this. If you discover any buggy behavior please let me know.
I believe it may have been inherited from the upstream author when referencing Collection.Pool (which also has this bug) into their own codebase.
I think this might originate from the usage of ArrayPool. And/or maybe I'd copied from the upstream's main branch instead of a stable .NET release.
it seems unnecessary to separately implement a ref series of collections apart from the Value series.
Actually I created the Temp- to enforce a specific policy. You're right it is somewhat unnecessary. I will think about this.