Making my own hash array.

Making my own hash array.

Hi,
I'm trying to use tbb::concurrent_hash_map to implement a wrapper class in order to use it in my programs. So far, I have done this:

#include "tbb/concurrent_hash_map.h"
#include 
template

struct HashCompare {

    static size_t hash(

        const K& key

    ) {

        return tbb::tbb_hasher(key);

    }
    static size_t hash(

      K* key

    ) {

      return reinterpret_cast( key/sizeof(key) );

    }
    static bool equal(

        const K& key1,

        const K& key2

    ) {

        return ( key1 == key2 );

    }

};
template

class HashArray {

    private:

      static unsigned int initial_hash_size;

      tbb::concurrent_hash_map > h;

      const HashArray& operator=(

          const HashArray& S

      );

    public:

      HashArray(): h(initial_hash_size) {}

      HashArray(

        const HashArray& S

      ) :

        h( S.h )

      {}
      ~HashArray() {}

      const E& operator[](

          const K& key

      ) const{

        typename tbb::concurrent_hash_map >::const_accessor a;

        if ( !h.find(a,key) ) {

            h.insert(a,key);

        }

        return a->second;

      }

      E& operator[](const K& key){

        typename tbb::concurrent_hash_map >::accessor a;

        if ( !h.find(a,key) ) {

            h.insert(a,key);

        }

        return a->second;

      }
      int IsDefined(const K key) const {

        if (h.count(key)!=0) return 1;

        else return 0;

      }

      void UnDefine(const K key) {

        typename tbb::concurrent_hash_map >::accessor a;

        if( h.find(a,key) ) {

          h.erase(a);

        }

      }

      void GatherStats(std::ostream& O) const {

        typename tbb::concurrent_hash_map >::const_iterator it( h.begin() );

        while ( it != h.end() ) {

            O << ((*it).second).size();

            ++it;

        }

      }

      void Clear(void) { h.clear(); }

      void Resize(void) { h.clear(); }

};
template

unsigned int HashArray::initial_hash_size=1000;
I have tested this code in a simple program and it works well. However, I would like to hear what you think about it. What I fear is that it's bad code and it might fail in a large program.

Thanks!

4 帖子 / 0 全新
最新文章
如需更全面地了解编译器优化,请参阅优化注意事项

Hello? Anyone?

Anton Malakhov (Intel)的头像

Hi,Sorry, it is completely wrong approach. The bad code is marked in comments belowQuotingkio.marv

I'm trying to usetbb::concurrent_hash_mapto implement a wrapper class in order to use it in my programs.

Usually it is bad idea to wrap concurrent_hash_map as an STL-like serial container. It leads to fatal threading errors.

    static size_t hash(
      K* key

Why a pointer? shouldn't it be a reference instead?

      return reinterpret_cast( key/sizeof(key) );

Are you aware that the actual key is a pointer to a key value? E.g. if you have two instances of the same value, they will be considered as different keys.

    static bool equal( 
        const K& key1, 
        const K& key2 
    ) {
        return ( key1 == key2 );
    }

However, here, you are comparing the values of keys, not pointers.

      const E& operator[](
          const K& key
      ) const{
        typename tbb::concurrent_hash_map >::const_accessor a;
        if ( !h.find(a,key) ) {
            h.insert(a,key);
        } 
        return a->second;
      }

find() is excessive as insert does the same job.
And it is not thread-safe as you return the reference without protection by accessor while using erase().

      E& operator[](const K& key){

Concurrent modifications are also not protected.

      void UnDefine(const K key) {
        typename tbb::concurrent_hash_map >::accessor a;
        if( h.find(a,key) ) {
          h.erase(a);
        }
      }

Well, it is the right way to prevent other threads from working with a concurrently erased instance. But I guess it was not the intention.. because without correctly used accessors, find() is excessive here as well.

I have tested this code in a simple program and it works well. However, I would like to hear what you think about it. What I fear is that it's bad code and it might fail in a large program.

You are right.

Hi everybody,

Quoting kio.marv...I have tested this code in a simple program and it works well. However, I would like to hear what you think about it. What I fear is that it's bad code and it might fail in a large program...
Here are a couple of questions:

- Did you try to evaluateperformance ofyourcodes and tocomparewith performance ofHash classesofSTL or Boost libraries?

- Did you do a Stress Testing with as larger as possibledata sets?

- Could you submit a Test-Case?

Best regards,
Sergey

登陆并发表评论。