Use cached PropertyNodes in python API for property reads

[seanmcleod]

Based on performance tests in #977 implement the PropertyNode caching in the Python API for property reads.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a5855b6) 24.87% compared to head (ce47e42) 24.87%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #993   +/-   ##
=======================================
  Coverage   24.87%   24.87%           
=======================================
  Files         168      168           
  Lines       18908    18908           
=======================================
  Hits         4704     4704           
  Misses      14204    14204           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[bcoconni]

So Python key hashing is faster than SGPropertyNode name lookup ? 😄

Nice patch 👍, just a tiny comment: I think the dictionary should have a more explicit name such as properties_cache.

[seanmcleod]

Yep, key hashing definitely faster than property name lookup, plus we were doing 2 property name lookups/traversals. Initially via pm.hasNode(_key) which was discarding the node pointer and simply returning a bool, and then another property name lookup during the call to GetPropertyValue(name). Plus I guess a couple of transitions from python code to JSBSim C++ code and back as well.

    def __getitem__(self, key: str) -> float:
        _key = key.strip()
        pm = self.get_property_manager()
        if not pm.hasNode(_key):
            raise KeyError("No property named {}".format(_key))
        return self.get_property_value(_key)

    def get_property_value(self, name: str) -> float:
        """@Dox(JSBSim::FGFDMExec::GetPropertyValue) """
        return self.thisptr.GetPropertyValue(name.encode())

[bcoconni]

Yes the code of __getitem__ was implemented before FGPropertyNode was included in the Python binding. At that time, the problem was that get_property_value always returns a value whether the property exists or not. So to inform users if they were requesting the value of a property that did not exist, the existence was first tested via has_node prior to calling get_property_value.

Indeed, now that FGPropertyNode is available the sequence of testing the existence of the property then returning the value of that property is now feasible with a single name lookup, which is faster. And it is even faster with a cache. Your patch hits 2 birds with a stone 🐦 👍

[seanmcleod]

Ah, was wondering why it was coded that way.

I'm used to the more aggressive form, "to kill two birds with one stone" 😉

[bcoconni]

I'm used to the more aggressive form, "to kill two birds with one stone" 😉

Well, I'll try to pretend my error was made on purpose because aviation as a whole needs to be friendlier to the environment 😄