document a weakness of boost::hash for strings
This commit is contained in:
parent
7fd28e23eb
commit
77548e74c6
5 changed files with 139 additions and 5 deletions
|
|
@ -82,12 +82,16 @@ namespace asset {
|
|||
* conjunction with LUID. How to create a LuidH instance, if not generating
|
||||
* a new random value. How to make EntryID and asset::Ident interchangeable, /////////TICKET #739
|
||||
* which would require both to yield the same hash values....
|
||||
* @warning there is a weakness in boost::hash for strings of running numbers, causing
|
||||
* collisions already for a small set with less than 100000 entries.
|
||||
* To ameliorate the problem, we hash the symbol twice /////////TICKET #865
|
||||
* @warning this code isn't portable and breaks if sizeof(size_t) < sizeof(void*)
|
||||
*/
|
||||
inline LuidH
|
||||
buildHash (string const& sym, HashVal seed =0)
|
||||
{
|
||||
boost::hash_combine(seed, sym);
|
||||
boost::hash_combine(seed, sym); ////////////////////////TICKET #865
|
||||
lumiera_uid tmpLUID;
|
||||
lumiera_uid_set_ptr (&tmpLUID, reinterpret_cast<void*> (seed));
|
||||
return reinterpret_cast<LuidH&> (tmpLUID);
|
||||
|
|
@ -173,6 +177,11 @@ namespace asset {
|
|||
* EntryID template share a common baseclass, usable for type erased common registration.
|
||||
* @todo currently storing the symbolic-ID as string. It should be a lib::Symbol,
|
||||
* but this isn't possible unless we use a symbol table. //////TICKET #158
|
||||
* @warning there is a weakness in boost::hash applied to strings. EntryID by default
|
||||
* generates symbolic IDs from a type prefix plus a running number, which causes
|
||||
* collisions already with less than 100000 entries.
|
||||
* @todo As a temporary workaround, we hash the symbolic ID twice, but we should look
|
||||
* into using a better hash function ////////////TICKET #865
|
||||
*
|
||||
* @see mobject::session::Track
|
||||
*/
|
||||
|
|
|
|||
|
|
@ -356,6 +356,11 @@ return: 0
|
|||
END
|
||||
|
||||
|
||||
TEST "HashGenerator_test" HashGenerator_test <<END
|
||||
return: 0
|
||||
END
|
||||
|
||||
|
||||
TEST "HashIndexed_test" HashIndexed_test <<END
|
||||
return: 0
|
||||
END
|
||||
|
|
|
|||
|
|
@ -38,6 +38,7 @@
|
|||
using lib::test::showSizeof;
|
||||
using lib::test::randStr;
|
||||
using util::isSameObject;
|
||||
using util::contains;
|
||||
using util::and_all;
|
||||
using lib::Literal;
|
||||
using std::string;
|
||||
|
|
@ -257,22 +258,28 @@ namespace test {
|
|||
{
|
||||
Hashtable tab;
|
||||
|
||||
for (uint i=0; i<1000; ++i) /////////////////////////////////////TICKET #587 we get strange collisions for 10000 entries
|
||||
for (uint i=0; i<10000; ++i) //////////////////TICKET #865 hash collisions for 100000 entries
|
||||
{
|
||||
DummyID dummy;
|
||||
tab[dummy] = string(dummy);
|
||||
}
|
||||
|
||||
CHECK (1000 == tab.size());
|
||||
|
||||
CHECK (and_all (tab, verifyEntry));
|
||||
CHECK (10000 == tab.size());
|
||||
}
|
||||
|
||||
|
||||
static bool
|
||||
verifyEntry (Hashtable::value_type entry)
|
||||
{
|
||||
return entry.second == string(entry.first);
|
||||
return checkForHashCollision(string(entry.first), entry.second);
|
||||
}
|
||||
|
||||
static bool
|
||||
checkForHashCollision(string const& key, string const& val)
|
||||
{
|
||||
if (key != val) cout << "Hash collision: "<<key<<" != "<<val<< endl;
|
||||
return key == val;
|
||||
}
|
||||
|
||||
};
|
||||
|
|
|
|||
113
tests/lib/hash-generator-test.cpp
Normal file
113
tests/lib/hash-generator-test.cpp
Normal file
|
|
@ -0,0 +1,113 @@
|
|||
/*
|
||||
HashGenerator(Test) - hash value generation details
|
||||
|
||||
Copyright (C) Lumiera.org
|
||||
2011, Hermann Vosseler <Ichthyostega@web.de>
|
||||
|
||||
This program is free software; you can redistribute it and/or
|
||||
modify it under the terms of the GNU General Public License as
|
||||
published by the Free Software Foundation; either version 2 of
|
||||
the License, or (at your option) any later version.
|
||||
|
||||
This program is distributed in the hope that it will be useful,
|
||||
but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
GNU General Public License for more details.
|
||||
|
||||
You should have received a copy of the GNU General Public License
|
||||
along with this program; if not, write to the Free Software
|
||||
Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
|
||||
|
||||
* *****************************************************/
|
||||
|
||||
|
||||
#include "lib/test/run.hpp"
|
||||
#include "lib/util.hpp"
|
||||
|
||||
#include <boost/functional/hash.hpp>
|
||||
#include <boost/lexical_cast.hpp>
|
||||
#include <iostream>
|
||||
#include <string>
|
||||
#include <map>
|
||||
|
||||
using boost::lexical_cast;
|
||||
using util::contains;
|
||||
using std::string;
|
||||
using std::cout;
|
||||
using std::endl;
|
||||
|
||||
|
||||
namespace lib {
|
||||
namespace test{
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
/***************************************************************************
|
||||
* @test cover various detail aspects regarding hash value generation
|
||||
* - weakness of boost::hash
|
||||
*
|
||||
* @see HashIndexed_test
|
||||
* @see EntryID_test
|
||||
*/
|
||||
class HashGenerator_test : public Test
|
||||
{
|
||||
|
||||
virtual void
|
||||
run (Arg)
|
||||
{
|
||||
demonstrate_boost_hash_weakness();
|
||||
}
|
||||
|
||||
|
||||
/** @test demonstrate a serious weakness of boost::hash for strings.
|
||||
* When hashing just the plain string representation of integers,
|
||||
* we get collisions already with small numbers below 100000.
|
||||
* This is counter-intuitive, as the generated hash values
|
||||
* are 17 digits long and could span much wider scale.
|
||||
*
|
||||
* This problem is especially dangerous when storing objects keyed
|
||||
* by a string-id, which is generated from running numbers.
|
||||
*/
|
||||
void
|
||||
demonstrate_boost_hash_weakness ()
|
||||
{
|
||||
typedef boost::hash<string> BoostStringHasher;
|
||||
typedef std::map<size_t, string> StringsTable;
|
||||
|
||||
BoostStringHasher hashFunction;
|
||||
StringsTable hashValues;
|
||||
string prefix = "Entry.";
|
||||
uint collisions(0);
|
||||
for (uint i=0; i<100000; ++i)
|
||||
{
|
||||
string candidate = prefix + lexical_cast<string> (i);
|
||||
size_t hashVal = hashFunction(candidate);
|
||||
|
||||
if (contains (hashValues, hashVal))
|
||||
{
|
||||
++collisions;
|
||||
string other = hashValues[hashVal];
|
||||
cout << "Duplicate at "<< i << endl;
|
||||
cout << "existing--->" << other << endl;
|
||||
cout << "new-------->" << candidate << endl;
|
||||
|
||||
size_t exHash = hashFunction(other);
|
||||
size_t newHash = hashFunction(candidate);
|
||||
cout << "hash-ex---->" << exHash << endl;
|
||||
cout << "hash_new--->" << newHash << endl;
|
||||
}
|
||||
hashValues[hashVal] = candidate;
|
||||
}
|
||||
CHECK (0 < collisions, "boost::hash for strings is expected to produce collisions");
|
||||
}
|
||||
|
||||
};
|
||||
|
||||
|
||||
/** Register this test class... */
|
||||
LAUNCHER (HashGenerator_test, "unit common");
|
||||
|
||||
|
||||
}} // namespace lib::test
|
||||
|
|
@ -88,7 +88,7 @@ namespace test{
|
|||
FixedFrameQuantiser fixQ(25);
|
||||
|
||||
uint frames = (rand() % MAX_FRAMES);
|
||||
FSecs dirt = (F25 / (rand() % DIRT_GRAIN));
|
||||
FSecs dirt = (F25 / (1 + rand() % DIRT_GRAIN));
|
||||
|
||||
Time rawTime (dirt + frames*F25);
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue