-
-
Notifications
You must be signed in to change notification settings - Fork 398
[MATLAB] Improve performance in outofprocess mode #2042
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
Conversation
`getArray` and `getString` now use persistent cache to avoid repeated allocation of buffers. Some infrequently updated properties in `Kinetics`, `Mixture`, and `ThermoPhase` are now cached. These cached properties are reset when relevant methods that could change them are called.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@ ## main #2042 +/- ## ========================================== + Coverage 76.41% 76.42% +0.01% ========================================== Files 463 463 Lines 54952 55004 +52 Branches 9308 9308 ========================================== + Hits 41990 42038 +48 - Misses 9831 9835 +4 Partials 3131 3131 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ssun30 - thanks for looking into this. I had a cursory glance at this PR.
In hindsight, it is obvious that those repeated integer fetches would create a lot of latency. Buffering them makes a whole lot of sense! At the same time, I believe that you can simplify the logic quite a bit by assuming that these integers only change for certain operations, i.e., constructor and anything that resizes. Let's take the example of Kinetics.nReactions. I recommend simplifying to
function n = get.nReactions(obj)
n = obj.kinCache.nReactions;
end
and move the logic
obj.kinCache.nReactions = ct.impl.call('mKin_nReactions', obj.kinID);
to the constructor, or any method that modifies the number of reactions. I have a strong hunch that this can be simplified even further by using regular MATLAB integers instead of the struct you introduced for caching.
Regarding the array cache, it may be simpler to create them as hidden members of objects (ThermoPhase, Kinetics, etc.), as each of the classes just use one or two standard lengths. Passing that cache to ctArray would be sufficient, and eliminate the logic to select/create the correct cache size there. You can obviously hybridize this with what you already have to take care of edge cases. Fwiw, CLib doesn't care about receiving arrays that are larger than necessary.
Uh oh!
There was an error while loading. Please reload this page.
Changes proposed in this pull request
This PR introduces several optimizations to the MATLAB interface that reduces execution time for the out-of-process mode, especially in reactor examples using MATLAB's own ode solver.
getArray,getString), eliminating repeated allocation overhead inclib.arrayobjects.ct.Kinetics,ct.ThermoPhase, andct.Mixturefor frequently accessed constants (nPhases,nReactions,nSpecies,nElements, etc.).ThermoPhasequantities (atomicWeights,molecularWeights,charges,minTemp,maxTemp).ismembercan be slow in MATLAB).If applicable, fill in the issue number this pull request is fixing
Partially addressed #2034
If applicable, provide an example illustrating new features this pull request is introducing
Profiling comparison (Mac OS 15.7, Apple M2 chip):
profiler_pfr_unoptimized_outofprocess profiler_pfr_optimized_outofprocess
profiler_pfr_unoptimized_inprocess profiler_pfr_optimized_inprocessplug_flow_reactorexecution time reduced by ~60% inoutofprocessmode, but still an order of magnitude longer thaninprocessmode. These optimizations also speed upinprocessmode by a lesser extent, but time spent ongetArrayandcallstill reduced by ~30%.These are just a small subset of properties and methods that could be cached. They have a hit-rate of 100% until we allow the MATLAB interface to add species/reactions (but even with those the hit rate will be close to 100%). There are plenty elsewhere but I don't think the user would be calling them repeatedly. I'm still investigating whether setter methods with array inputs (such as
set.X) can be optimized with minimized memory copy.Checklist
scons build&scons test) and MATLAB examples execute successfully