Review Request 124539: Refactor KDevHash to use boost::hash_combine approach internally.

[Kevin Funk]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124539/#review83340
-----------------------------------------------------------


+1 for using "proved" hashing algorithms.

I always feel uncomfortable seeing random magic numbers in hash functions, where the origin of these values are not comprehensible from context.


language/util/kdevhash.h (line 45)
<https://git.reviewboard.kde.org/r/124539/#comment57597>

    const&?



language/util/kdevhash.h (line 53)
<https://git.reviewboard.kde.org/r/124539/#comment57598>

    Better document where this function is from (link to the boost doc)


- Kevin Funk


On Aug. 2, 2015, 2:16 p.m., Milian Wolff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124539/
> -----------------------------------------------------------
> 
> (Updated Aug. 2, 2015, 2:16 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> Instead of hashing every byte of an integral type individually,
> we leverage qHash and boost::hash_combine to create an overall
> hash. The result should be just as good, as it's proven technology,
> but the hash functions themselves become much faster. This is
> demonstrated by the newly added benchmark.
> 
> Before:
> 
>     ********* Start testing of TestKDevHash *********
>     Config: Using QtTest library 5.5.0, Qt 5.5.0 (x86_64-little_endian-lp64 shared (dynamic) release build; by GCC 5.1.0)
>     PASS   : TestKDevHash::initTestCase()
>     PASS   : TestKDevHash::benchHash_int()
>     RESULT : TestKDevHash::benchHash_int():
>          0.075 msecs per iteration (total: 77, iterations: 1024)
>     PASS   : TestKDevHash::benchHash_uint()
>     RESULT : TestKDevHash::benchHash_uint():
>          0.075 msecs per iteration (total: 77, iterations: 1024)
>     PASS   : TestKDevHash::benchHash_quint64()
>     RESULT : TestKDevHash::benchHash_quint64():
>          0.15 msecs per iteration (total: 77, iterations: 512)
>     PASS   : TestKDevHash::benchHash_bool()
>     RESULT : TestKDevHash::benchHash_bool():
>          0.018 msecs per iteration (total: 77, iterations: 4096)
>     PASS   : TestKDevHash::cleanupTestCase()
>     Totals: 6 passed, 0 failed, 0 skipped, 0 blacklisted
>     ********* Finished testing of TestKDevHash *********
> 
> After:
> 
>     ********* Start testing of TestKDevHash *********
>     Config: Using QtTest library 5.5.0, Qt 5.5.0 (x86_64-little_endian-lp64 shared (dynamic) release build; by GCC 5.1.0)
>     PASS   : TestKDevHash::initTestCase()
>     PASS   : TestKDevHash::benchHash_int()
>     RESULT : TestKDevHash::benchHash_int():
>          0.0244 msecs per iteration (total: 100, iterations: 4096)
>     PASS   : TestKDevHash::benchHash_uint()
>     RESULT : TestKDevHash::benchHash_uint():
>          0.024 msecs per iteration (total: 99, iterations: 4096)
>     PASS   : TestKDevHash::benchHash_quint64()
>     RESULT : TestKDevHash::benchHash_quint64():
>          0.025 msecs per iteration (total: 53, iterations: 2048)
>     PASS   : TestKDevHash::benchHash_bool()
>     RESULT : TestKDevHash::benchHash_bool():
>          0.020 msecs per iteration (total: 85, iterations: 4096)
>     PASS   : TestKDevHash::cleanupTestCase()
>     Totals: 6 passed, 0 failed, 0 skipped, 0 blacklisted
>     ********* Finished testing of TestKDevHash *********
> 
> REVIEW: 124539
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 70174433c9d9b88595254887c4f06c12aa7043e0 
>   language/CMakeLists.txt 2c2b0285b68d209e07af185c43dcf157507f5a54 
>   language/util/kdevhash.h 40c32935aec9c7e5552769e780c033e96cf63166 
>   language/util/tests/CMakeLists.txt PRE-CREATION 
>   language/util/tests/test_kdevhash.cpp PRE-CREATION 
>   plugins/projectfilter/projectfilter.h 014e5a1eabbb3a2d9e71baeee3e38643cb8649e7 
>   serialization/tests/test_indexedstring.cpp f0c7958ac33c6fc6c4febbaa106e0ca30f8a60f8 
> 
> Diff: https://git.reviewboard.kde.org/r/124539/diff/
> 
> 
> Testing
> -------
> 
> compiled and ran tests, everything seems to work fine
> 
> 
> Thanks,
> 
> Milian Wolff
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20150802/52e647be/attachment-0001.html>