[Performance] Running list of improvements for performance reasons

[jmarrec]

Enhancement Request

This issue is to centralize some (fairly simple) actionable items in order to improve performance. These are basically things that will avoid doing extra work or doing extra copies.

• Avoid push_backs when possible

  • by direct initializing vectors that aren't dynamic
    • example:

      OpenStudio/src/model/Space.cpp

      Lines 264 to 282 in 65607ae

      	std::vector<IddObjectType> Space_Impl::allowableChildTypes() const {
      	std::vector<IddObjectType> result;
      	result.push_back(IddObjectType::OS_ShadingSurfaceGroup);
      	result.push_back(IddObjectType::OS_InteriorPartitionSurfaceGroup);
      	result.push_back(IddObjectType::OS_Surface);
      	result.push_back(IddObjectType::OS_Lights);
      	result.push_back(IddObjectType::OS_Luminaire);
      	result.push_back(IddObjectType::OS_People);
      	result.push_back(IddObjectType::OS_ElectricEquipment);
      	result.push_back(IddObjectType::OS_ElectricEquipment_ITE_AirCooled);
      	result.push_back(IddObjectType::OS_GasEquipment);
      	result.push_back(IddObjectType::OS_HotWaterEquipment);
      	result.push_back(IddObjectType::OS_Daylighting_Control);
      	result.push_back(IddObjectType::OS_IlluminanceMap);
      	result.push_back(IddObjectType::OS_SpaceInfiltration_DesignFlowRate);
      	result.push_back(IddObjectType::OS_SpaceInfiltration_EffectiveLeakageArea);
      	result.push_back(IddObjectType::OS_SpaceInfiltration_FlowCoefficient);
      	return result;
      	}

    • In some occasion, maybe using a constexpr std::array would be better (Need to check impact on ruby code, and also some types like IddObjectType aren't suitable for use in a constexpr context as they aren't 'literal')
  • by using emplace_back to construct elements in place where possible
    • Lots of that in the FT for eg:

     - IdfObject idealLoadsAirSystem(IddObjectType::HVACTemplate_Zone_IdealLoadsAirSystem);
     - m_idfObjects.push_back(idealLoadsAirSystem);
     + auto& idealLoadsAirSystem = m_idfObjects.emplace_back(IddObjectType::HVACTemplate_Zone_IdealLoadsAirSystem);

• Make sure there are no functions left that take std::string (or any other non POD types) by value without reason (there are a couple of cases where we are mutating the string) to avoid extra copies: for your pImpl case, that's at least TWO extra copies for each call. There are close to a thousand occurrences left in just model/

  • OpenStudio/src/model/SizingSystem.hpp

    Line 240 in 65607ae

    bool setHeatingDesignAirFlowMethod(std::string heatingDesignAirFlowMethod);

  • OpenStudio/src/model/SizingSystem_Impl.hpp

    Line 229 in 65607ae

    bool setHeatingDesignAirFlowMethod(std::string heatingDesignAirFlowMethod);

• Same thing for setters taking a boost::optional<T> argument and checking if the argument is present...

  • it's inefficient and doesn't make much sense.
  • It's actually inconsistent: sometimes we reset sometimes we don't.

  • OpenStudio/src/model/ChillerElectricEIR.cpp

    Lines 315 to 321 in 65607ae

    	bool ChillerElectricEIR_Impl::setReferenceCapacity(boost::optional<double> referenceCapacity) {
    	bool result = false;
    	if (referenceCapacity) {
    	result = setDouble(OS_Chiller_Electric_EIRFields::ReferenceCapacity, referenceCapacity.get());
    	}
    	return result;
    	}

  • OpenStudio/src/model/CoolingTowerVariableSpeed.cpp

    Lines 321 to 331 in 65607ae

    	bool CoolingTowerVariableSpeed_Impl::setDesignInletAirWetBulbTemperature(boost::optional<double> designInletAirWetBulbTemperature) {
    	bool result(false);
    	if (designInletAirWetBulbTemperature) {
    	result = setDouble(OS_CoolingTower_VariableSpeedFields::DesignInletAirWetBulbTemperature, designInletAirWetBulbTemperature.get());
    	} else {
    	resetDesignInletAirWetBulbTemperature();
    	result = true;
    	}
    	return result;
    	}

  • This is technically an API break, though most users won't even notice it (I doubt many people actually try to pass optional doubles / strings)

• Modify all for (T t : vectorOfTs) to for(const T& t: vectorOfTs) if possible or for (T& t: vectorOfTs) if mutating it, to avoid extra copies on loops. The FT is ridden with this for eg:

  OpenStudio/src/energyplus/ForwardTranslator.cpp

  Lines 240 to 245 in 65607ae

  	for (Space space : model.getConcreteModelObjects<Space>()) {
  	if (!space.thermalZone()) {
  	LOG(Warn, "Space " << space.name().get() << " is not associated with a ThermalZone, it will not be translated.");
  	space.remove();
  	}
  	}

[shorowit]

• Reduce input validation more generally inside OpenStudio? This was, for example, the solution used to address the slow performance of the addViewFactor method.
• More formally integrate epJSON into OpenStudio so that generating epJSON is not an extra step relative to generating IDF, and therefore inherently slower.

[shorowit]

• Option 3 from Improve handling of invalid values passed to SDK #4477 (i.e., eliminate or defer input validation)?

[shorowit]

• Further enable use of CBOR instead of SQLite? Add cbor or msgpack gem to CLI #4496
• Connect OpenStudio to the EnergyPlus API instead of calling the EnergyPlus CLI to eliminate I/O (e.g., pass the model in memory to EnergyPlus, retrieve the outputs in memory from EnergyPlus); though it looks like this would require changes to EnergyPlus too?

[tijcolem]

• Fix #4456 - Improve performance of OpenStudio::UnzipFile::extractAllFiles #4493 to the list.
• Test SDK with Ruby v.3.0

[tijcolem]

Before we start to chip away at the this list, we'll want to establish a base test case to measure performance gains and quantify the result in some meaningful way. @jmarrec Are there some specific regression tests we can use to for this?

[shorowit]

• Add extralite gem to CLI as an alternative to sqlite3-ruby? (Note: We've moved over to msgpack but other users may benefit.)

[tijcolem]

Closing as this is complete. #4532