-
Notifications
You must be signed in to change notification settings - Fork 206
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve ValuePlug::getValue()
performance
#5362
Commits on Jul 11, 2023
-
ValuePlug : Avoid extra refcount operations
This produces the following reductions in runtime : - ValuePlugTest.testCacheOverhead : 7% - ValuePlugTest.testContentionForOneItem : 25%
Configuration menu - View commit details
-
Copy full SHA for 16487c9 - Browse repository at this point
Copy the full SHA 16487c9View commit details -
ValuePlug : Avoid unnecessary
typeId()
callThe common case is that a plug has no input, so it's worth avoiding the overhead of calling `typeId()`. This knocks ~2% off the runtime of `ValuePlugTest.testCacheOverhead`.
Configuration menu - View commit details
-
Copy full SHA for d961b0e - Browse repository at this point
Copy the full SHA d961b0eView commit details -
NumericPlug : Inline
getValue()
This produces the following reductions in runtime : - ValuePlugTest.testCacheOverhead : 5% - ValuePlugTest.testContentionForOneItem : 13%
Configuration menu - View commit details
-
Copy full SHA for 3f6f8e2 - Browse repository at this point
Copy the full SHA 3f6f8e2View commit details -
ValuePlug : Avoid additional refcount operations on cache hit
This produces the following reductions in runtime : - ValuePlugTest.testCacheOverhead : 7% - ValuePlugTest.testContentionForOneItem : 20%
Configuration menu - View commit details
-
Copy full SHA for 7803459 - Browse repository at this point
Copy the full SHA 7803459View commit details -
Plug : Inline
direction()
andnode()
This produces the following reductions in runtime : - ValuePlugTest.testCacheOverhead : 5% - ValuePlugTest.testContentionForOneItem : 1%
Configuration menu - View commit details
-
Copy full SHA for e170cdc - Browse repository at this point
Copy the full SHA e170cdcView commit details -
Configuration menu - View commit details
-
Copy full SHA for ff34605 - Browse repository at this point
Copy the full SHA ff34605View commit details -
Typed[Object]Plug : Move
.inl
toImplementation.h
We use `.inl` files to store inline implementation that we want the compiler to see, but which we don't want to clutter up the header with, so the headers are more human-readable. These files don't match that pattern - they're to be included in a single `.cpp` file and used to explicitly instantiate the template.
Configuration menu - View commit details
-
Copy full SHA for 89bbb74 - Browse repository at this point
Copy the full SHA 89bbb74View commit details -
Typed[Object]Plug : Inline
getValue()
callThis also means declaring AtomicFormatPlug's specialisations publicly so that we don't get the publicly visible default `getValue()` implementation being compiled instead. And the part I don't understand : we need to link `libGafferImage` to far more libraries now, to avoid undefined symbols related to AtomicFormatPlug with MSVC. It seems that the mere existence of the inlined `getValue()` - even though it is invalidated by the specialization - is enough to make MSVC greedily do a bunch of implicit template instantiations that it wasn't doing before, and which ultimately depend on `GafferImage::Format`.
Configuration menu - View commit details
-
Copy full SHA for 0945ec9 - Browse repository at this point
Copy the full SHA 0945ec9View commit details