martinus / map_benchmark

Comprehensive benchmarks of C++ maps
MIT License
299 stars 29 forks source link

Cleanup build by making folly build optional and adding aggregate map and hash builds #24

Closed camel-cdr closed 1 year ago

camel-cdr commented 1 year ago

Makes folly build optional (can be enabled with -DENABLE_FOLLY=TRUE) and adds aggregate <HASH>_all and <MAP>_all targets.

If you just want to evaluate a hash map against other hash maps, then building all hashes often isn't necessary, so having an aggregate target is quite helpful. Similarly, evaluating a hash implementation doesn't require you to build all hash tables.

camel-cdr commented 1 year ago

Tangentially: I was thinking of moving the USE_POOL_ALLOCATOR code into a wrapper of the hash maps that support it, so it doesn't clutter the benchmark code.

There are two options to do this, but both alter existing behavior:

  1. default constructor constructs resource: This would only change the behavior of the Copy benchmark, as the resource couldn't be shared between the two maps.
  2. default constructor uses static resource: This would change the behavior of all benchmarks except the Copy benchmark.

I think option 1. is more desirable, but I'm not sure if changing the Copy benchmark is behavior is ok.

In the end, it's just a cosmetic change to keep the benchmark code more clean. Is this a desirable change, and should I create a PR for it?

martinus commented 1 year ago

Thanks for that! folly continues to be problematic, good to disable it by default.

I'd leave the USE_POOL_ALLOCATOR for now as it is, yes the benchmarks are a bit cluttered but at least it is explicit what is happening