2

毎日何か新しいことを学ぼうとしている私は、次のデザインが良いか悪いか興味があります。

Aそれ自体のオブジェクトを static private member variable にキャッシュするクラスを実装していますstd::map<> cache。のユーザーはA、マップ内の要素へのポインターにのみアクセスできるようにするA必要があります。新しいAは、マップでまだ利用できない場合にのみ作成されますA。わかりました、ここにいくつかのコードがあります：

class B;

class A {
    public:
        static A* get_instance(const B & b, int x) {
            int hash = A::hash(b,x);
            map<int, A>::iterator found = cache.find(hash);
            if(found == cache.end())
                found = cache.insert(make_pair(hash, A(b,x))).first;
            return &(found->second);
        }
        static int hash(B & b, int x) { 
            // unique hash function for combination of b and x
        }
        // ...

    private:
        A(B & b, int x) : _b(b), _x(x) { 
            // do some heavy computation, store plenty of results 
            // in private members
        }
        static map<int, A> cache;
        B _b;
        int _x; // added, so A::hash() makes sense (instead of B::hash())
        // ...
 };

上記のコードに何か問題がありますか? 落とし穴はありますか、メモリ管理の問題などを見逃すことはありませんか?

ご意見ありがとうございます！

4

3 に答える 3

3

この実装は、get_instance() を介してのみアイテムを作成できるようにすることを目的としています。理想的には、コピー コンストラクターと代入演算子を非公開にする必要があります。

スレッドセーフではありません。代わりに以下を使用できます。

const boost::once_flag BOOST_ONCE_INIT_CONST = BOOST_ONCE_INIT;

struct AControl
{
  boost::once_flag onceFlag;
  shared_ptr<A> aInst;

  void create( const B&b, int x )
  {
      aInst.reset( new A(b, x) );
  }

  AControl() : onceFlag( BOOST_ONCE_INIT_CONST )
  {
  }

  A& get( const B&b, int x )
  {
     boost::call_once( onceFlag, bind( &AOnceControl::create, this, b, x ) );
     return *aInst;
  }
};

マップをマップに変更

ミューテックスを用意して、次のように使用します。

AControl * ctrl;
{
  mutex::scoped_lock lock(mtx);
  ctrl = &cache[hash];
}
return ctrl->get(b,x);

理想的には、 get_instance() のみがクラスで静的になります。それ以外はすべて非公開の実装の詳細であり、AControl を含むクラスのコンパイル ユニットに入ります。

マップを検索して作成するプロセス全体をロックするだけで、これをはるかに簡単に行うことができますが、長い構築プロセスを実行している間、ロックする時間が長くなることに注意してください。アイテムを挿入すると、レコードレベルのロックが実装されます。boost::once後のスレッドで、項目が初期化されていないことがわかる場合がありますが、ロジックによって項目が1 回だけ作成されることが保証されます。

于 2011-03-01T15:44:19.597 に答える
1

これらは、A内で混ぜ合わせる3つの別々のものだと思います。

  • クラスA自体（そのインスタンスが行うことになっていること）。
  • キャッシュを目的としたインスタンスのプーリング
  • 特定のタイプのためのそのような静的な単一のプールを持っている

A内ですべて一緒にではなく、コード内で別々にする必要があると思います。

つまり、次のことを意味します。

  • クラスAは、どのように割り当てるかを考慮せずに記述してください。

  • 次の行に沿って、オブジェクトのプールキャッシュを実行する汎用モジュールを記述します。

*

template< typename T > class PoolHashKey { ... };

template< typename T > class PoolCache  
{  
//data  
  private: std::map< .... > map_;  

//methods  
    public: template< typename B > PoolKey< T > get_instance( B b );  
    public: void release_instance( PoolKey< T > );  
    // notice that these aren't static function members  
};
  • どこかにPoolCacheのシングルトンインスタンスを作成し、それを使用します。

*

PoolCache<A>& myAPool()  
{  
    static PoolCache<A> s;  
    return s;  
    //you should use some safe singleton idiom.  
}  

int main()  
{  
  B b;  
  PoolKey<A> const aKey( myAPool().get_instance( b );  
  A* const a( aKey.get() );  
  //...  
  myAPool().release_instance( aKey ); //not using it anymore
  /*or else the destructor of PoolKey<A> should probably do some reference count and let the pool know this instace isn't needed anymore*/
}
于 2011-03-01T16:21:27.253 に答える
1

グローバル (この場合は静的マップ) を使用するときはいつでも、これが複数のスレッドで使用される場合、同時実行の問題について心配する必要があります。たとえば、2 つのスレッドが特定のインスタンスを一度に取得しようとすると、両方がオブジェクトを作成して重複が発生する可能性があります。さらに悪いことに、両者が同時にマップを更新しようとすると、マップが破損する可能性があります。コンテナーへのアクセスを制御するには、ミューテックスを使用する必要があります。

シングルスレッドのみの場合、誰かが将来マルチスレッドにする必要があると判断するまで問題はありません。

また、スタイル ノートとして、アンダースコア + 小文字で始まる名前は技術的には合法ですが、アンダースコアで始まる記号を避けることで、誤って規則に違反したり、奇妙な動作をしたりする可能性を回避できます。

于 2011-03-01T15:43:55.590 に答える