FFIFunctionEndPoint Execute() performance refactor #9619
Conversation
| //The dsObject must be of pointer type | ||
| Validity.Assert(dsObject.IsPointer || dsObject.IsFunctionPointer, | ||
| string.Format("Operand type {0} not supported for marshalling", dsObject.optype)); | ||
| Validity.Assert(dsObject.IsPointer || dsObject.IsFunctionPointer, "Operand type not supported for marshaling"); |
There was a problem hiding this comment.
Could we add the message to the string resources in the project.
|
2 more comparisons made (with fast config):
It can be seen that after removing the stack refactoring improvement, the Run performance degrades as much as 20% for one of the graphs. |
|
@saintentropy has agreed to give this one more look and rerun perf benchmarks on the PR and comparison with baseline to determine the improvements if any. I suggested he use the default config and run only the model tests to save time. @saintentropy you can refer to this wiki to learn how to use the latest perf benchmarking tool options: https://wiki.autodesk.com/pages/viewpage.action?spaceKey=GEN&title=Dynamo+Performance+Guidelines |
* Move repetative intitialization in Execute to Init method * Pass stack values directly to FFI execute vs using the stack data structure * Change error string to a static value vs a calculated string * Modify list allocation in ExecWithZeroRI for better performance * some cleanup * restore old methods, mark them obsolete
Purpose
The purpose of this PR is a follow up to the enhancement in #8755 where the VM detects list data structures with homogeneous member types and short circuits duplicative creation of redundant
FFIFunctionEndPointobjects (FFI = Foreign Function Interface). Based on that change, this PR refactors theExecute()method withinFFIFunctionEndPointto move its initialization logic to anInit()that is only called once vs every call. The refactor also removes a dependency on using the Stack data structure to pass stack values to the eventual invoke mechanism. In the case of FFI there is no possibility of local variables being pushed back to the Stack so this use case was a redundant duplication of the existing stack values. Lastly this PR changes how theCallSite.StackValue ExecWithRISlowPath()clones replication data structures as well as stack value arrays with more efficient mechanisms.Performance comparison with PR improvements (Base) and master (New) (ran model tests on standard perf test suite using fast config):
Performance comparison after PR (Base) and master (New) using default config:
The second comparison is confusing as it does not reflect the expected perf improvements as seen in the first even though it is expected to be more accurate given that it was run with the default config. It suggests that there isn't much of an improvement at all?
Declarations
Check these if you believe they are true
*.resxfilesReviewers
@mjkkirschner @QilongTang @reddyashish I [Aparajit] have added a commit to obsolete the older API's so as not to break API compatibility and also cleaned up the PR a bit.
FYIs
(FILL ME IN, Optional) Names of anyone else you wish to be notified of