Initial Implementation of DenseUniqueValueMap #122
Conversation
DenseUniqueValueMap is used to replace the OneToManyDict that is being used in `ClientManager`. Given that `ClientManager` is the only user of the original data structure, this commit aims to provide an alternative to OneToManyDict that is less generic but more compact in terms of RAM usage. DenseUniqueValueMap does not provide all the APIs that OneToManyDict provides. Instead, it provides only the API that is being used by the user (aka. `ClientManager`). There are 4 APIs in total. Current implementation of DenseUniqueValueMap is not final. There are many things missing, for example proper `typedef`s are needed. NOTE: There is another API that is being used in `ClientManager`. That is `self._client_to_task_ids.get_values(client)`. The caller to this API, however, is not being used in scaler. Perhaps it should be remove later. Signed-off-by: gxu <[email protected]>
| #include <map> | ||
| #include <vector> | ||
|
|
||
| struct DenseUniqueValueMap { |
There was a problem hiding this comment.
This could be a template I think.
There was a problem hiding this comment.
This should be a template, and will be a template at a later point (i.e. When we agree to do more work on this). The signature should be something this,
template <Comparable K, DefaultConstructible V, /* possibly allocator specialization */ class A>
class DenseUniqueValueMap
But @sharpener6 puts it on hold. So that will be done later.
| } | ||
|
|
||
| auto [kIter, _] = keyToRefCount.insert_or_assign(key, ++keyToRefCount[key]); | ||
| it->second = kIter; |
There was a problem hiding this comment.
Are you sure kIter (should be a size_t * if I understand it correctly) will not be invalidated at some point?
e.g. if std::map does some internal resizing, could it move the size _t value to another location? I've the feeling you're correct and that it should not happen because std::map is tree-based (std::unordered_map definitively does it). But I couldn't find a clear statement in the documentations.
There was a problem hiding this comment.
Looks like this is fine. Cppreference.com does not say anything about it, but cplusplus.com seems to confirm that only .erase() invalidates iterators.
There was a problem hiding this comment.
Non-contiguous containers preserves validity of Iterators over resizing. Rehashing is another case, std::unordered_map invalidates iterators when rehashing.
2 participants
DenseUniqueValueMap is used to replace the OneToManyDict that is being used in
ClientManager. Given thatClientManageris the only user of the original data structure, this commit aims to provide an alternative to OneToManyDict that is less generic but more compact in terms of RAM usage.DenseUniqueValueMap does not provide all the APIs that OneToManyDict provides. Instead, it provides only the API that is being used by the user (aka.
ClientManager). There are 4 APIs in total.Current implementation of DenseUniqueValueMap is not final. There are many things missing, for example proper
typedefs are needed.NOTE: There is another API that is being used in
ClientManager. That isself._client_to_task_ids.get_values(client). The caller to this API, however, is not being used in scaler. Perhaps it should be remove later.