ValuePlug : Avoid unnecessary reference counting #5520
Conversation
When `getObjectValue()` returns `m_staticValue`, the caller does not need to own a reference, because it is guaranteed to be kept alive by the plug itself. The caller only needs to own a reference to computed values, which may not be cached, or may be evicted from the cache on a secondard thread. This yields the following performance improvements : - testHashCacheOverhead (GafferTest.ValuePlugTest.ValuePlugTest) : was 1.15s now 1.05s (9% reduction) - testStaticNumericValuePerformance (GafferTest.ValuePlugTest.ValuePlugTest) : was 0.76s now 0.01s (99% reduction) - testStaticObjectValuePerformance (GafferTest.ValuePlugTest.ValuePlugTest) : was 0.78s now 0.72s (8% reduction) - testStaticStringValuePerformance (GafferTest.ValuePlugTest.ValuePlugTest) : was 0.95s now 0.01s (99% reduction)
|
This all looks good to me. As for your question about IntrusiveOrNotPtr - I feel like whether that would be worthwhile or not depends on whether there are other places we'd want to use |
I can't think of one at the moment, and I think it might be quite tricky to get the semantics and usage clear enough for general consumption. I did look around to see if there was a pre-rolled one in |
|
I don't see anything wrong with leaving this as is. |
When
getObjectValue()returnsm_staticValue, the caller does not need to own a reference, because it is guaranteed to be kept alive by the plug itself. The caller only needs to own a reference to computed values, which may not be cached, or may be evicted from the cache on a secondard thread.This yields the following performance improvements :
@danieldresser-ie, I did wonder about creating some sort of intrusive/raw pointer variant to return from
getObjectValue()rather than passing theownerin separately. I think such a thing might make the call sites slightly simpler, but since this is really only likely to be used internally I wasn't sure it was worth the extra code. Let me know what you think...