NASA-AMMOS / aerie

A software framework for modeling spacecraft.
https://nasa-ammos.github.io/aerie-docs/
MIT License
73 stars 19 forks source link

Naming efficiency #1486

Closed DavidLegg closed 5 months ago

DavidLegg commented 5 months ago

Description

Several proposed performance optimizations for the Aerie streamline framework, all indicated by profiling done on the CADRE simplified data model. In particular:

  1. Changes many more string operations done when computing names to be lazy; those string operations are only done if a name is requested. Since names are primarily a debugging concept, in the vast majority of cases, names are assigned but never requested, making this a significant performance improvement.
  2. Refrains from adding duplicate constraints to the active queue in LinearBoundaryConsistencySolver, limiting re-work when solving.
  3. Optimizes Expiry.or to use Optional methods instead of converting to Streams. This is less elegant but much more performant.
  4. Using effectively immutable lists for Context, so that daemon tasks can share references with their parent task's context rather than copying and rebuilding the list. Since daemon tasks contribute most of the load of context management, it makes sense to optimize for their primary use case.
  5. Re-writing the clampedIntegrate function to directly calculate its results, rather than leaning on the LinearBoundaryConsistencySolver. While this requires some tinkering with lower-level concepts, it roughly doubles the speed of clampedIntegrate. For models that use this function, it can be a performance bottleneck.

Verification

All changes were tested by hand on the CADRE simplified data model.

That model, running a 1 day plan, went from taking a few minutes to taking ~15s, with all the changes in this PR. Also, it looks like ~5s of that time is spent in simulation, and the remaining ~10s are spent saving the results.

Documentation

N/A - pure refactor

Future work

N/A

Note: This PR makes #1483 and #1405 obsolete.

dandelany commented 5 months ago

Thanks @DavidLegg for the massive performance improvement and @mattdailis for reviewing! Merging so that we can include this in the 2.14 release. FYI I have to "resolve" the conversations above in order to merge, but feel free to unresolve & continue the convo if there's more to be said.