0

I read a lot about unordered_map not being very fast but I wonder what's the best alternative to do this:

I need to buffer calculation results for a function of an integer argument. I don't know ahead of time what range or interval will be requested. Storing in a vector with maximal resolution would cost way too much memory. So I'm using unordered_map<unsigned long, pair<T, long>> Where the key is the argument of the function to be computed, the first of the pair the result of the computation of type T, and the second of the pair a version information for that computation. Only if the unordered_map does not contain the element or it contains it but the version is outdated, the computation is carried out and then added to the unordered_map. The lookup function looks something like this:

template<typename T> class BufferClass{
  long MyVersion;
  unordered_map<unsigned long, pair<T,long>> Buffer;
public:
  BufferClass(): MyVersion{1} {};

  T* GetIfValid(unsigned long index)
  {
    if (!Buffer.count(index)) return nullptr;
    pair <T,long> &x{Buffer.at(index)};
    if (x.second!=MyVersion) return nullptr;
    return &x.first;
  }
/* ...Functions to set elements...*/
}

As you can see, I combined element validity check and retrieval in one function, so that I only need one lookup for both.

The profiler shows most of the computation time is used up in the hash function __constrain_hash related to unordered_map.

What would be the fastest way to store and retrieve values like that? The list of stored indices is expected to be non-continuous (there will be a lot of "holes") and first and last index are also mostly unknown.

T will generally be a "small" data type (like double or complex).

Thanks!

Martin

HolyBlackCat
  • 78,603
  • 9
  • 131
  • 207
Martin
  • 21
  • 1
  • 1
    "*The profiler shows most of the computation time is used up in the hash function __constrain_hash related to unordered_map.*" ... and? The profilers will always show that most of the computation time is used up *somewhere*. The question is whether too much time is being spent. So... is it? How often do you call it, and how many such calls are you expecting to do in one second? – Nicol Bolas Mar 01 '21 at 07:23
  • there exist flat map implementations which can perform better than unordered_map under very specific usage patterns, but they all come with tradeoffs and limitations. in general, stick with general purpose hash maps e.g. std::unordered_map (and fix your usage of it to be more optimal, as noted in the Answers) – Patrick Parker Mar 01 '21 at 07:52

2 Answers2

2

In your code, there could be two hash lookup in one query, one invoked in count() and the other invoked in at(). It is redundant, use unordered_map::find instead, see here.

Sample code:

const auto iter = Buffer.find(index);
if(iter != Buffer.end()) //Found something, so the return value is not end()
{
    return &(iter->first);
}
else return nullptr;

In my opinion, unordered_map is slow but not that slow, for 99.9% usage is fast enough. You may want to check whether you call this function (unnecessarily) too many times. Using other fast implementation is not free, it could bloat your code base, harm your application's compatibility with different host systems or so on. If you think std::unordered_map is unreasonably slow, it is almost always because you got somewhere wrong in your work. (either your estimation or your code implementation)

BTW, another thing to mention: You said T is a small data type right? then return its value instead of pointer to it, it is faster and safer.

LIU Qingyuan
  • 524
  • 5
  • 18
0

One thing that strikes me as odd about your implementation is the following two lines:

    if (!Buffer.count(index)) return nullptr;
    pair <T,long> &x{Buffer.at(index)};

This code is checking if the key exists, then throws away the result and searches for the same key again with bounds checking to boot. I think you'll find searching once with std::unordered_map<unsigned long, std::pair<T, long>>::find and reusing the result to be preferable:

    auto it = Buffer.find(index);
    if (it == Buffer.end()) return nullptr;
    auto& x = *it;
Patrick Roberts
  • 49,224
  • 10
  • 102
  • 153
  • "*with bounds checking to boot*" Bounds checking for an unordered map doesn't cost anything; either it found the item or it didn't. So using `at` only adds conditional `throw`. – Nicol Bolas Mar 01 '21 at 07:30
  • @NicolBolas it's an extra branch at best. They're claiming that `std::unordered_map` is too slow. If they're that concerned about performance, they should be removing unecessary branching logic as well. – Patrick Roberts Mar 01 '21 at 07:33