junction icon indicating copy to clipboard operation
junction copied to clipboard

Added the insert operation

Open mdashti opened this issue 7 years ago • 1 comments

This pull-request adds an insert operation (which simply fails when a mapping already exists). This operation simplifies the usage and avoids unnecessary locking on the user side and it specially helps when the value is dynamically allocated (as opposed to the solutions provided in http://preshing.com/20160201/new-concurrent-hash-maps-for-cpp).

mdashti avatar Jan 17 '18 11:01 mdashti

Hi,

Thanks for your work! I think I see where your point.

Just a few problems with the pull request:

  • There are no tests. A new feature like this warrants a .cpp file for a new test in MapCorrectnessTests Ideally the test would simulate many threads racing to insert dynamically allocates and validate that there are no leaks or double frees.
  • For this feature I'd prefer the names tryInsert and tryInsertValue instead of insert and putValueIfNotExists. (I realize std::map defines insert but I'm not modeling after std::map, and "try" is a little more descriptive.)
  • The TURF_TRACE macros should be prefixed with [Mutator::tryInsertValue] as well (not [Mutator::exchangeValue]) and the trace indices should be cleaned up with turf/extra/TidySource.py. We should also make sure the tests exercise every codepath when run. I realize the trace system hasn't been documented anywhere; I could provide more info if you want.
  • Would be good to factor out these lines to a common function.
  • Feature not implemented in the Linear or Leapfrog maps.

preshing avatar Jan 17 '18 14:01 preshing