Improve performance of SliceTransform::AsString #9401
Conversation
1. Removed the options from the Capped/Fixed SliceTransforms. Instead these classes are created with id.number. This allows the GetID() id to be calculated and stored at class construction time 2. Improve the performance of AsString by using the ID only if there are no option properties (which is the case for all of the builtin transforms). Ran tests of calling AsString in a loop 5M times and found approximately a 10x performance increase vs the original code.
|
@mrambacher has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
By making the function virtual and requiring implementations with options to override it, performance improves to close to parity with prior implementations.
|
@mrambacher has updated the pull request. You must reimport the pull request before landing. |
Summary: A proposed alternate fix to performance regression associated with SliceTransform::AsString, vs. facebook#9401 Test Plan: performance testing TODO
include/rocksdb/slice_transform.h
Outdated
| // and any additional properties. | ||
| // The default implementation is optimized for instances which have no | ||
| // additional properties/options. Instances which have properties/options | ||
| // should override this method and call ToString(); |
There was a problem hiding this comment.
I would prefer not to involve the API extension user in performance concerns in non-obvious ways.
(See #9407)
|
@mrambacher has updated the pull request. You must reimport the pull request before landing. |
|
@mrambacher has updated the pull request. You must reimport the pull request before landing. |
|
@mrambacher has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@mrambacher has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@mrambacher This seems to have broken the LITE build; could you please take a look? |
Removed the options from the Capped/Fixed SliceTransforms. Instead these classes are created with id.number. This allows the GetID() id to be calculated and stored at class construction time. This change puts the construction back to similar to how it was prior to the Customizable changes for SliceTransform.
Improve the performance of AsString by using the ID only if there are no option properties (which is the case for all of the builtin transforms).
Ran tests of calling AsString in a loop 5M times and found approximately a 10x performance increase vs the original code.