opensim-org / opensim-core

SimTK OpenSim C++ libraries and command-line applications, and Java/Python wrapping.
https://opensim.stanford.edu
Apache License 2.0
759 stars 308 forks source link

Rewrite PropertyTable to use std::unordered_map #3722

Closed adamkewley closed 4 months ago

adamkewley commented 4 months ago

Reimplements PropertyTable to store the string-to-index lookup in an std::unordered_map, rather than a std::map.

The motivation for doing this is model editing performance. PropertyTable, Component, Socket, etc. are all using std::map, which is almost always implemented as a red-black tree. This means that copying a Model (e.g. because an editor wants to back up an old version) spends a large amount of time copying tree nodes, strings, etc.

Changing the lookup to an O(1) lookup also has the advantage of making runtime property lookups faster

I will try to build a downstream fork of OSC with this branch to get an idea of where the performance impact is, but I figure that it's a nice refactor to do in general.

Brief summary of changes

Testing I've completed

Looking for feedback on...

CHANGELOG.md (choose one)


This change is Reviewable

adamkewley commented 4 months ago

Downstream benchmarking indicates that this doesn't have a big impact on model copying, so this is more of a feel-good change ;)

adamkewley commented 4 months ago

Thanks for checking it out @nickbianco - too bad it doesn't have any particularly big gains, but hopefully it's an otherwise nice little clean-up!

nickbianco commented 4 months ago

@adamkewley definitely worth a shot, and yes, a nice refactor regardless :)