Release XML element when property value node was bound

[cbirkhold]

Analog to GetNode().

This significantly reduces memory cost by not keeping a large number of XML elements around in the common case of successfully binding the property in the constructor.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 24.91%. Comparing base (90755f7) to head (4d8951d).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1096   +/-   ##
=======================================
  Coverage   24.91%   24.91%           
=======================================
  Files         168      168           
  Lines       18108    18110    +2     
=======================================
+ Hits         4511     4513    +2     
  Misses      13597    13597           

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

[seanmcleod]

This significantly reduces memory cost by not keeping a large number of XML elements...

Did you take any memory measurements to compare?

Out of interest I tried a couple of tests to see how much of a memory difference there is. I used Visual Studio 2022 and took a look at some memory figures with and without this change. I ran the 737_cruise.xml script.

Before:

After:

[cbirkhold]

In my test case I'm seeing a 25% reduction in runtime memory cost. Your example is a bit less then that but still a nice win for essentially a one liner that simply removes unused memory. While the absolute value appears small, in memory constraint environments every little helps. This also removes clutter when memory debugging in other areas.

[seanmcleod]

Out of interest, are you using JSBSim in some memory constrained environment?

[cbirkhold]

Yes.

I'm approaching 256 kb runtime load. This change has the highest memory saving over lines changed ratio but if there is interest I can extract a few more that are still transparent.

[seanmcleod]

Out of interest what environment are you running JSBSim in?

[bcoconni]

Good catch @cbirkhold !

BTW the assertion you've added makes me think that there are most likely some places in the code where the value returned by FGPropertyManager::GetNode is not checked.

[cbirkhold]

@bcoconni The assert I added almost as an explanation to why the follow line is 'legal'. The HasNode() check above is the key really.

BTW the assertion you've added makes me think that there are most likely some places in the code where the value returned by FGPropertyManager::GetNode is not checked.

I think the code is generally pretty good on that in either setting the create flag to true or calling HasNode() before. Some cases call Tie() before (which only logs an error but does not return a status or throw an exception) and assume that succeeds so that's a potential hazard (for instance FGTurbine::bindmodel()).