Optimize calls to `SGPropertyNode::getNode` in `FGAtmosphere::Calculate` by bcoconni · Pull Request #1382 · JSBSim-Team/jsbsim

bcoconni · 2026-02-01T14:24:30Z

The problem

As I mentioned in #1375 (comment), the method FGAtmosphere::Calculate is spending a significant amount of time resolving property names into property nodes (i.e. calling SGPropertyNode::getNode):

Actually the logic is based on the existence of the nodes atmosphere/override/temperature, atmosphere/override/pressure and atmosphere/override/density.

jsbsim/src/models/FGAtmosphere.cpp

Lines 142 to 170 in 5988bb4

    
           void FGAtmosphere::Calculate(double altitude) 
        
           { 
        
             SGPropertyNode* node = PropertyManager->GetNode(); 
        
             double t =0.0; 
        
             if (!PropertyManager->HasNode("atmosphere/override/temperature")) 
        
               t = GetTemperature(altitude); 
        
             else 
        
               t = node->getDoubleValue("atmosphere/override/temperature"); 
        
             Temperature = ValidateTemperature(t, "", true); 
        
             double p = 0.0; 
        
             if (!PropertyManager->HasNode("atmosphere/override/pressure")) 
        
               p = GetPressure(altitude); 
        
             else 
        
               p = node->getDoubleValue("atmosphere/override/pressure"); 
        
             Pressure = ValidatePressure(p, "", true); 
        
             if (!PropertyManager->HasNode("atmosphere/override/density")) 
        
               Density = Pressure/(Reng*Temperature); 
        
             else 
        
               Density = node->getDoubleValue("atmosphere/override/density"); 
        
             Soundspeed  = sqrt(SHRatio*Reng*Temperature); 
        
             PressureAltitude = CalculatePressureAltitude(Pressure, altitude); 
        
             DensityAltitude = CalculateDensityAltitude(Density, altitude); 
        
             Viscosity = Beta * pow(Temperature, 1.5) / (SutherlandConstant + Temperature); 
        
             KinematicViscosity = Viscosity / Density; 
        
           }

If one of these nodes is existing then the temperature/pressure/density is extracted from its corresponding node. This feature is designed to use an external atmospheric model that would provide its data through the atmosphere/override properties.

The problem is that interrogating SGPropertyNode::getNode at each iteration is costly: 72012 calls to SGPropertyNode::getNode translates into 3,940,012 calls to strncmp (in the screenshot above it is an SSE2 optimized version of it).

The proposed solution

Since all the nodes are located below atmosphere/override then the new code is only testing if atmosphere/override is existing at each iteration. If it does not exist then there is no point in interrogating the other nodes for the temperature/pressure/density. This is dividing the number of calls to SGPropertyNode::getNode by 3 and the time spent in FGAtmosphere::Calculate by roughly the same order of magnitude.

In addition to that, once the existence of a node is confirmed, it is registered in a variable of type SGPropertyNode_ptr so that FGAtmosphere::Calculate will no longer need to resolve the name into a node thus avoiding further calls to SGPropertyNode::getNode.

codecov · 2026-02-01T14:27:27Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 25.29%. Comparing base (5988bb4) to head (85eb24d).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1382      +/-   ##
==========================================
+ Coverage   25.24%   25.29%   +0.05%     
==========================================
  Files         169      169              
  Lines       18549    18563      +14     
==========================================
+ Hits         4682     4696      +14     
  Misses      13867    13867

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

… `Calculate`.

bcoconni · 2026-02-01T15:31:49Z

I have added one further improvement: the node for atmosphere is now saved in a class member atmosphere_node so that, we are interrogating the override existence below the atmosphere_node rather than atmosphere/override.

This is saving 14% calls to strncmp, not that much of an improvement but while I was it, I thought I could go that step further.

bcoconni · 2026-02-01T15:37:20Z

Just for the sake of comparing time execution, on my (quite old) PC the script scripts/c1723.xml is running in ~1.45s with the master branch and ~1.4s with this PR. That's an improvement of 3%-4% to the execution speed. So not that much of a deal but well, that's better than nothing.

seanmcleod70 · 2026-02-01T19:06:52Z

the new code is only testing if atmosphere/override is existing at each iteration.

I was wondering about the necessity of each iteration when I was looking at the commit, then noticed back here that you specifically mentioned wanting to check on each iteration. I guess when reviewing the code I assumed the override properties would exist at startup, but I guess someone can create them at any time, and therefore we need to check on each iteration?

bcoconni · 2026-02-02T10:25:35Z

Indeed I have made this PR targeting 100% backward compatibility without questioning whether or not that would make sense. That's a good point you are making: do we really need to check on every iteration if the property has been created ?

One option could be to check when FGFDMExec::RunIC() is called i.e. when we can assume that the system is completely set up. But that would mean breaking backward compatibility. However I am pretty sure this overriding feature is unknown to the vast majority of the users, if not all of them. So it may not be that much of problem.

seanmcleod70 · 2026-02-02T19:50:01Z

Yep, I wasn't aware of this atmosphere override option, so wasn't really sure if there was a standard for when the override property needed to be created etc. So yes, safest option for now in terms of backwards compatibility is to go with your current implementation. A 3%-4% increase is still a pretty useful improvement.

bcoconni · 2026-02-03T08:41:47Z

Yep, I wasn't aware of this atmosphere override option,

Neither was I. I have discovered or, should I say, paid attention to it when investigating issue #1375

So yes, safest option for now in terms of backwards compatibility is to go with your current implementation. A 3%-4% increase is still a pretty useful improvement.

OK. The PR has been merged.

…te` (JSBSim-Team#1382)

Optimize calls to SGPropertyNode::getNode(const char*, ...)

0a37275

Avoid checking the existence of the atmosphere node at each call to…

85eb24d

… `Calculate`.

bcoconni force-pushed the optimize_SGPropertyNode_calls branch from ade9a23 to 85eb24d Compare February 1, 2026 15:00

bcoconni merged commit 05ddea2 into JSBSim-Team:master Feb 3, 2026
28 checks passed

bcoconni deleted the optimize_SGPropertyNode_calls branch February 3, 2026 08:38

bcoconni added a commit to bcoconni/jsbsim that referenced this pull request Feb 6, 2026

Optimize calls to SGPropertyNode::getNode in `FGAtmosphere::Calcula…

e053f7b

…te` (JSBSim-Team#1382)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize calls to `SGPropertyNode::getNode` in `FGAtmosphere::Calculate`#1382

Optimize calls to `SGPropertyNode::getNode` in `FGAtmosphere::Calculate`#1382
bcoconni merged 2 commits intoJSBSim-Team:masterfrom
bcoconni:optimize_SGPropertyNode_calls

bcoconni commented Feb 1, 2026

Uh oh!

codecov bot commented Feb 1, 2026 •

edited

Loading

Uh oh!

bcoconni commented Feb 1, 2026

Uh oh!

bcoconni commented Feb 1, 2026 •

edited

Loading

Uh oh!

seanmcleod70 commented Feb 1, 2026 •

edited

Loading

Uh oh!

bcoconni commented Feb 2, 2026

Uh oh!

seanmcleod70 commented Feb 2, 2026

Uh oh!

Uh oh!

bcoconni commented Feb 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	void FGAtmosphere::Calculate(double altitude)
	{
	SGPropertyNode* node = PropertyManager->GetNode();
	double t =0.0;
	if (!PropertyManager->HasNode("atmosphere/override/temperature"))
	t = GetTemperature(altitude);
	else
	t = node->getDoubleValue("atmosphere/override/temperature");
	Temperature = ValidateTemperature(t, "", true);

	double p = 0.0;
	if (!PropertyManager->HasNode("atmosphere/override/pressure"))
	p = GetPressure(altitude);
	else
	p = node->getDoubleValue("atmosphere/override/pressure");
	Pressure = ValidatePressure(p, "", true);

	if (!PropertyManager->HasNode("atmosphere/override/density"))
	Density = Pressure/(Reng*Temperature);
	else
	Density = node->getDoubleValue("atmosphere/override/density");

	Soundspeed = sqrt(SHRatioRengTemperature);
	PressureAltitude = CalculatePressureAltitude(Pressure, altitude);
	DensityAltitude = CalculateDensityAltitude(Density, altitude);

	Viscosity = Beta * pow(Temperature, 1.5) / (SutherlandConstant + Temperature);
	KinematicViscosity = Viscosity / Density;
	}

Conversation

bcoconni commented Feb 1, 2026

The problem

The proposed solution

Uh oh!

codecov bot commented Feb 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bcoconni commented Feb 1, 2026

Uh oh!

bcoconni commented Feb 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

seanmcleod70 commented Feb 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bcoconni commented Feb 2, 2026

Uh oh!

seanmcleod70 commented Feb 2, 2026

Uh oh!

Uh oh!

bcoconni commented Feb 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Feb 1, 2026 •

edited

Loading

bcoconni commented Feb 1, 2026 •

edited

Loading

seanmcleod70 commented Feb 1, 2026 •

edited

Loading