DPL / DPL Analysis: Introduce LabeledArray as an option for Variant #5313
Conversation
aalkin
commented
Jan 27, 2021
aalkin
commented
- extended Variant
- added JSON/ptree conversions
- added cases to workflow serializer/deserializer
- added tests
- added example
* extened Variant * added JSON/ptree conversions * added cases to workflow serializer/deserializer * added tests
| Array2D<T> values; | ||
| std::vector<std::string> labels_rows; | ||
| std::vector<std::string> labels_cols; | ||
| std::unordered_map<std::string, uint32_t> rowmap; |
There was a problem hiding this comment.
do you really need the unordered_map?
There was a problem hiding this comment.
It was the easiest way to make label getter work without too much performance drop. I will try to rework this later.
There was a problem hiding this comment.
Did you check what was the actual performance issue? I strongly believe that in get(std::string, std::string) the most time consuming part is allocating the strings, not the lookup. Why not using get(char const*, char const*)?
There was a problem hiding this comment.
Getter arguments should've been references... The issue was that using std::find on string vector was slower than map lookup even for sizes below 10. And since typical usage would be getting non-sequential values of the array to use in multiple if statements in process() that could be a problem. But I do not like using unordered_map and intend to rewrite it with the hash trick we use in registries.
There was a problem hiding this comment.
even if they are references, if the common usage is get("foo", "bar"), as I assume it is, it will do two allocations for each invocation (because char const* is not a string). At least you should provide an overload, if that is a typical usecase.
|
Merged to speed up developments. Could you rework stuff so that the unordered_map is not needed and the methods parts which do not require templated code are factored out in a non template (private) policy? |
2 participants
