Follow up to #44261: string_id performance and correctness

[Aivean]

Summary

SUMMARY: Infrastructure "Improve string_id comparison performance, add unit test and benchmarks"

Purpose of change

Follow up to #44261, improvements were suggested by @kevingranade and @jbytheway :

• make string_id::_version 64 bit
• avoid magic "-1" version value in generic factory in the unlikely scenario when version overflows and wraps around

Describe the solution

Implemented suggestions regarding version. In addition:

• improved performance of string_id comparison when both string_ids have same versions (see benchmarks below)
• added unit tests and benchmarks

Describe alternatives you've considered

1. I was also thinking about the case when one or both string_ids that participate in equality check have outdated version.
   For example, this might happen when one string_id is static const that is used only for comparison with some dynamic entities.

   In such case it would make sense to update cache of both ids (by invoking string_id::id() method) before comparison. However, currently string_id::id() is optional and might not exist. I'm not sure if there is some C++ template magic that can check in compile time whether string_id::id() is implemented for particular string_id type so we can have different string_id::operator== implementation depending on string_id::id() existence.

   A simpler approach would be to manually override string_id::operator== for string_id types where string_id::id() is defined and add cache update there.

2. Currently I've added commented out CATCH_CONFIG_ENABLE_BENCHMARKING definition to the CMakeLists.txt and Makefile. Probably it would be better to have it enabled conditionally as an option or a build parameter. Would it be appropriate to add a global option to cmake build, something like "BENCHMARKS"? I'm not sure about that, because this option only makes sense for the test build.

Testing

Checked that game compiles and works.
Added unit tests that check string_id equality correctness in all possible scenarios.

Additional context

I've benchmarked the performance of string_id::obj lookup with 32bit and 64bit version and the difference was within the error margin.

Also, below are benchmark results for string comparison before and after change:

Before

0.000 s: short id

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cata_test is a Catch v2.13.0 host application.
Run with -? for options

Randomness seeded to: 1600834019

-------------------------------------------------------------------------------
string_id_compare_benchmark
-------------------------------------------------------------------------------
../tests/generic_factory_test.cpp:131
...............................................................................

benchmark name                       samples       iterations    estimated
                                     mean          low mean      high mean
                                     std dev       low std dev   high std dev
-------------------------------------------------------------------------------
ids are equal, invalid version                 100          5196     4.1568 ms
                                        9.18487 ns    8.76305 ns    9.68308 ns
                                        1.76242 ns    1.52797 ns    2.38175 ns

ids are not equal, invalid version             100          6037     4.2259 ms
                                        7.37708 ns    7.37173 ns    7.41019 ns
                                      0.0477637 ns 0.00425835 ns   0.129494 ns

ids are equal, valid version                   100          5424     4.3392 ms
                                        7.98667 ns    7.78428 ns    8.41975 ns
                                        1.10188 ns   0.555934 ns    1.79839 ns

ids are not equal, valid version               100          6178     4.3246 ms
                                        6.87691 ns    6.87408 ns     6.8932 ns
                                      0.0241352 ns 0.00191195 ns  0.0610316 ns



0.000 s: long id
-------------------------------------------------------------------------------
string_id_compare_benchmark
-------------------------------------------------------------------------------
../tests/generic_factory_test.cpp:131
...............................................................................

benchmark name                       samples       iterations    estimated
                                     mean          low mean      high mean
                                     std dev       low std dev   high std dev
-------------------------------------------------------------------------------
ids are equal, invalid version                 100          3442     4.4746 ms
                                         13.412 ns    13.2603 ns    14.2398 ns
                                        1.25196 ns   0.061067 ns    3.34365 ns

ids are not equal, invalid version             100          3393     4.4109 ms
                                        12.8335 ns    12.8308 ns      12.84 ns
                                      0.0153519 ns 0.00851935 ns  0.0312431 ns

ids are equal, valid version                   100          3492     4.1904 ms
                                        15.6175 ns      15.37 ns      16.12 ns
                                        1.31102 ns   0.810126 ns     2.4167 ns

ids are not equal, valid version               100          3401     4.4213 ms
                                        12.8682 ns    12.6435 ns    13.3673 ns
                                        1.24534 ns   0.660683 ns    2.14539 ns

After:

0.000 s: short id

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cata_test is a Catch v2.13.0 host application.
Run with -? for options

Randomness seeded to: 1600836221

-------------------------------------------------------------------------------
string_id_compare_benchmark
-------------------------------------------------------------------------------
../tests/generic_factory_test.cpp:131
...............................................................................

benchmark name                       samples       iterations    estimated
                                     mean          low mean      high mean
                                     std dev       low std dev   high std dev
-------------------------------------------------------------------------------
ids are equal, invalid version                 100          5661     3.9627 ms
                                        6.97096 ns    6.91936 ns    7.06244 ns
                                       0.261528 ns   0.163127 ns   0.381402 ns

ids are not equal, invalid version             100          6625      3.975 ms
                                        6.72076 ns    6.63882 ns    7.11076 ns
                                        0.65394 ns   0.111454 ns    1.82071 ns

ids are equal, valid version                   100         24724     2.4724 ms
                                        1.76077 ns    1.71926 ns    1.88528 ns
                                       0.254946 ns 0.00156317 ns   0.515177 ns

ids are not equal, valid version               100         21416     4.2832 ms
                                          2.078 ns    2.02011 ns    2.22853 ns
                                       0.337671 ns  0.0882099 ns   0.602139 ns


7.104 s: string_id_compare_benchmark
0.000 s: long id
-------------------------------------------------------------------------------
string_id_compare_benchmark
-------------------------------------------------------------------------------
../tests/generic_factory_test.cpp:131
...............................................................................

benchmark name                       samples       iterations    estimated
                                     mean          low mean      high mean
                                     std dev       low std dev   high std dev
-------------------------------------------------------------------------------
ids are equal, invalid version                 100          2883     4.3245 ms
                                        14.8302 ns    14.8166 ns     14.896 ns
                                       0.108377 ns  0.0137436 ns   0.285552 ns

ids are not equal, invalid version             100          3010      4.214 ms
                                        14.5322 ns    14.5272 ns    14.5648 ns
                                      0.0448823 ns 0.00406477 ns   0.114573 ns

ids are equal, valid version                   100         24109     2.4109 ms
                                        1.77016 ns    1.76963 ns    1.77385 ns
                                     0.00416943 ns 0.000510221 ns  0.0115821 ns

ids are not equal, valid version               100         21286     4.2572 ms
                                        2.03305 ns    2.00848 ns    2.16068 ns
                                        0.18526 ns  0.0174827 ns   0.450918 ns

Conclusion:
Before the change, on my machine, depending on the string length, string_id comparison took 6-7ns for short strings, and 12-15 ns for long strings.

After the change, there was no significant overhead when comparing string ids with outdated version, but when versions are valid, comparison time drops to 1.7-2ns regardless of the string length (i.e. 3x-6x speedup).

[ZhilkinSerg]

CATCH_CONFIG_ENABLE_BENCHMARKING should be also added to VS project (I've did this for you).