Open
Description
From anonymous:
Many compilers include hash_map class and now it is even in C++11 standard by the name unordered_map.
I propose another macro JSON_MAP_TYPE which will be defined to the best possible class.
Currently I can choose only JSON_USE_CPPTL_SMALLMAP or not.
It is not enough, I want to use map based on hash, it will be more efficient.
Activity
Dani-Hub commentedon Mar 7, 2015
Instead of introducing just another flag it seems to me preferable to test the compiler macro
__cplusplus
against the value that refers to (at least) C++11. Visual Studio compilers - even though they providestd::unordered_map
- can be tested against the value of _MSC_VER.The following preprocessor test presents a possible extension of the current one:
If interest exist to follow this direction I could try to provide a patch file.
cdunn2001 commentedon Mar 7, 2015
That sounds like a good idea in general.
However, we have also to worry about binary compatibility. If it's a flag, people can use it at their leisure. Otherwise, we have to be very careful about the sizes of data-structures. (It's not the map; it's the map iterators which we expose.)
Dani-Hub commentedon Mar 8, 2015
In this case I have a slightly different suggestion: For the C++11 development branch (1.5.x) of jsoncpp, I would suggest to replace unconditionally the reference to
<map>
by<unordered_map>
.cdunn2001 commentedon Mar 8, 2015
ObjectValues::Iterator
is a member ofValueIteratorBase
. (I'm not the fond of those iterators, but they exist.) Maybe it's not really a big deal to break binary-compatibility for the1.y.z
series, but let's see a benefit first. What is the improvement in runtime and memory?Dani-Hub commentedon Mar 8, 2015
A
std::map
will provide logarithmic complexities for it's operations, whilestd::unordered_map
in the average realizes constant complexity.cdunn2001 commentedon Mar 9, 2015
Hmmm. I really hate breaking binary-compatibility. People definitely instantiate Iterators in for-loops. It's hard to justify without a major-version bump. If we could say, "With this input, the savings are such and such," at least we would have an argument.
On the other hand, a major-version bump is not terrible. But maybe we should do more things along with this. E.g., the bump is the opportunity to drop the deprecated classes. So I'd rather not rush into this.
But I'm fine with a compiler-flag.
MartinBonner commentedon Nov 18, 2015
Binary compatibility: Also remember that at the moment maps return their members in alphabetical order. With a change to unordered_map, they wouldn't. Now obviously, JsonCpp doesn't promise to return members in order - but I bet that there is someone, somewhere, whose application will be broken if it stops doing so.
Which sounds to me like a very strong argument for a major version bump or a compiler flag.
Edit: Fix typo