Improve performance for file reading/writing #8996

mbadams5 · 2021-08-23T16:32:25Z

Pull request overview

This pull request grew from its initial scope to something quite large. This PR focused mostly on improving performance when reading and writing files, including Schedule:File, Schedule:File:Shading, EPW weather files, epJSON, and more.

This PR also improves performance in other portions of the code, such as strings to integers and doubles.

Another large change is moving FMT library from 6.1.2 to 8.0.1, which required understanding the custom Fortran formatting code and adapting it to the new FMT api for custom formatting.

The performance improvements can be seen in the tables below:

HybridZoneModel_shadow113.idf
	Develop (ms)	Branch (ms)	Runtime Reduction
Overall	41130	4190	89.8%
ProcessScheduleInput	35350	355	99.0%
ReadEPlusWeatherForDay	964	26	97.3%
InterpretWeatherDataLine	935	10	98.9%
commonRun	325	104	68.0%
WriteTimeStampFormatData	120	80	33.3%
WriteNumericData	71	57	19.7%
SetCurrentWeather	340	176	48.2%
ProcessNumber	528	1	99.8%

HybridZoneModel.idf
	Develop (ms)	Branch (ms)	Runtime Reduction
Overall	4400	3490	20.7%
ProcessScheduleInput	164	38	76.8%
ReadEPlusWeatherForDay	828	33	96.0%
InterpretWeatherDataLine	809	10	98.8%
commonRun	261	97	62.8%
WriteTimeStampFormatData	104	69	33.7%
WriteNumericData	68	62	8.8%
SetCurrentWeather	273	151	44.7%
ProcessNumber	36	3	91.7%

NOTE: ENHANCEMENTS MUST FOLLOW A SUBMISSION PROCESS INCLUDING A FEATURE PROPOSAL AND DESIGN DOCUMENT PRIOR TO SUBMITTING CODE

Pull Request Author

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

Reviewer

This will not be exhaustively relevant to every PR.

# Conflicts: # src/ConvertInputFormat/main.cpp # src/EnergyPlus/AirflowNetworkBalanceManager.cc # src/EnergyPlus/DaylightingManager.cc # src/EnergyPlus/GroundHeatExchangers.cc # src/EnergyPlus/HeatBalanceIntRadExchange.cc # src/EnergyPlus/InputProcessing/IdfParser.cc # src/EnergyPlus/InternalHeatGains.cc # src/EnergyPlus/OutputReportTabular.hh # src/EnergyPlus/OutputReports.cc # tst/EnergyPlus/unit/PVWatts.unit.cc

New schedule:file parser is more strict Changed "z" in LocalEnvData.csv to 0 to match previous implementation Gtest is more strict about exact matching or something now too for other unit test

This was causing issues with std::string_view and fmt to some extent This is a hacky solution for now to just create a string but it would be good to avoid the string creation if possible

It wasn't implicitly converting the iterator to a pointer This uses the data() call which is a const char * to accomplish the same effect

This was causing build errors on MSVC due to ambiguity in FMT and Objexx

# Conflicts: # src/EnergyPlus/InputProcessing/InputProcessor.cc

Apparently MSVC doesn't like this being unique_ptr since something, somewhere is terminating the pointer but then tries to delete it again.....? Anyways, changing this to shared_ptr since it won't delete until all references are 0. The original code was a memory leak (although probably not a bad one).

Myoldmopar

This is littered with great stuff. There are 836 files changed, so obviously I'm not going to be able to go line by line. The changes I have looked over look great though, and CI is very happy. I think this is ready to go.

Myoldmopar · 2021-09-03T13:09:39Z

scripts/dev/generate_epJSON_schema/idd_parser.py

@@ -152,7 +152,7 @@ class Data:


 def parse_idd(data):
-    root = {'$schema': "http://json-schema.org/draft-04/schema#", 'properties': {}}
+    root = {'$schema': "https://json-schema.org/draft-07/schema#", 'properties': {}}


What are the implications of changing to draft 7? Does it unlock more capabilities in the schema?

Yes, it does. There was only 1 minor change needed to update our schema to draft 7 (exclusiveMinimum and exclusiveMaximum are now values instead of boolean, basically you put either minimum or exclusiveMinimum, not both, now).

Myoldmopar · 2021-09-03T13:10:00Z

scripts/dev/generate_embeddable_epJSON_schema/generate_embeddable_epJSON_schema.cpp

@@ -19,7 +19,7 @@ int main( int argc, char const *argv[] )
 	auto const input_json = json::parse( schema_stream );
 	auto const v_cbor = json::to_cbor( input_json );

-	printf("	const static std::array< std::uint8_t, %zu > embeddedSchema = {{\n", v_cbor.size() );
+	printf("	static constexpr std::array< std::uint8_t, %zu > embeddedSchema = {{\n", v_cbor.size() );


Does this have any impact on performance or is it just more appropriate?

I don't think it affects performance, however, it can be used in constexpr functions now (It likely could have before, but I haven't tested or read to see if that is true)

Myoldmopar · 2021-09-03T13:10:17Z

src/EnergyPlus/Autosizing/Base.cc

@@ -312,9 +312,9 @@ void BaseSizer::reportSizerOutput(EnergyPlusData &state,
                                  Optional<Real64 const> UsrValue)
 {

-    static constexpr fmt::string_view Format_990(
+    static constexpr std::string_view Format_990(


Myoldmopar · 2021-09-03T13:10:39Z

src/EnergyPlus/AirLoopHVACDOAS.cc

@@ -798,7 +798,7 @@ namespace AirLoopHVACDOAS {
                }
                AirLoopMixer thisAirLoopMixer;
                thisDOAS.m_CompPointerAirLoopMixer = thisAirLoopMixer.factory(state, thisDOAS.m_AirLoopMixerIndex, thisDOAS.AirLoopMixerName);
-                thisDOAS.AirLoopSplitterName = UtilityRoutines::MakeUPPERCase(AsString(fields.at("airloophvac_splitter_name"))); //
+                thisDOAS.AirLoopSplitterName = UtilityRoutines::MakeUPPERCase(fields.at("airloophvac_splitter_name").get<std::string>()); //


Much nicer.

Myoldmopar · 2021-09-03T13:11:41Z

src/EnergyPlus/CurveManager.cc

+                    str.remove_prefix(first_char);
+                }
+                double result = 0;
+                auto answer = fast_float::from_chars(str.data(), str.data() + str.size(), result);


Myoldmopar · 2021-09-03T13:12:11Z

src/EnergyPlus/DataOutputs.hh

-        RE2 *pattern;
-        RE2 *case_insensitive_pattern;
+        std::shared_ptr<RE2> pattern = nullptr;
+        std::shared_ptr<RE2> case_insensitive_pattern = nullptr;


This will be helpful 👍

Myoldmopar · 2021-09-03T13:44:02Z

src/EnergyPlus/FileSystem.hh

+
+    template <FileTypes fileType> std::string getJSON(const nlohmann::json &data, int const indent = 4)
+    {
+        if constexpr (is_json_type(fileType)) {


Myoldmopar · 2021-09-03T13:47:10Z

src/EnergyPlus/General.cc

+    static constexpr std::array<std::string_view, NumSingleChars>SingleChars{"/", ":", "-"};
+    static constexpr int NumDoubleChars(6);
+    static constexpr std::array<std::string_view, NumDoubleChars> DoubleChars{"ST ", "ND ", "RD ", "TH ", "OF ", "IN "}; // Need trailing spaces: Want thse only at end of words
+    static constexpr std::array<std::string_view, 12> Months{"JAN", "FEB", "MAR", "APR", "MAY", "JUN", "JUL", "AUG", "SEP", "OCT", "NOV", "DEC"};


mbadams5 added 24 commits August 19, 2021 15:32

Update Milo dtoa and itoa libraries

3123c23

Update SQLite library 3.29.0->3.36.0

4d98dc1

Update Valijson ~v0.3 to v0.5

34de6de

Update RE2 2019-09-01 -> 2021-08-01

750cd59

Update GTest v1.10.0 -> v1.11.0

8cae782

Convert to make_unique

7cda1e1

Convert fmt::string_view to std::string_view

9b3e9be

Move FatalError to EnergyPlus.hh

c40bfd2

Upgrade FMT library 6.1.2 -> 8.0.1 (f69a572)

0b488c9

Update JSON Library 3.7.3 -> 3.10.0 (4336048)

6357467

Faster file reading

d3e767e

Faster file writing

8be90e9

Use fmt::ostream instead of std::ostream in certain spots

2071e2e

Faster string to double/float

ef9566a

Improve weather file parsing speed

9fcca1d

Assorted improvements

8a6451d

Improve schedule:file and schedule:file:shading reading

6415a62

Improve IdfParser

ae53ed0

Fix unit test build error

8efd844

Simplify ConvertInputFormat code and improve performance

115b369

Fix runtime bug in release build

556f1f3

Fix bugs and update some

12034b8

Merge remote-tracking branch 'origin/develop' into improved_file_reading

ac1fee9

mbadams5 added the Performance Includes code changes that are directed at improving the runtime performance of EnergyPlus label Aug 23, 2021

mbadams5 added this to the EnergyPlus 9.6 Release milestone Aug 23, 2021

mbadams5 requested a review from Myoldmopar August 23, 2021 16:33

mbadams5 assigned Myoldmopar and mjwitte Aug 23, 2021

Fix linux build errors

c4821b9

mbadams5 and others added 23 commits August 25, 2021 01:26

Fix clang-format error

e2f72f5

Fix various bugs in unit tests

85834be

Merge remote-tracking branch 'origin/develop' into improved_file_reading

22c5642

Fix more bugs

0b623ae

Fix unit tests

7189a3d

New schedule:file parser is more strict Changed "z" in LocalEnvData.csv to 0 to match previous implementation Gtest is more strict about exact matching or something now too for other unit test

filesystem::path uses wchar_t on windows but char on everything else

a417ed7

This was causing issues with std::string_view and fmt to some extent This is a hacky solution for now to just create a string but it would be good to avoid the string creation if possible

MSVC didn't like comparing const char * and std::string_view::end()

37c940e

It wasn't implicitly converting the iterator to a pointer This uses the data() call which is a const char * to accomplish the same effect

Fix build errors and other warnings

a675bae

Remove all using namespace ObjexxFCL; from unit tests

3c0138e

This was causing build errors on MSVC due to ambiguity in FMT and Objexx

Merge remote-tracking branch 'origin/develop' into improved_file_reading

05c7b85

Fix clang-format issues

0c96bd2

Latest library updates

aa2e3e4

Fix bug when no <charconv>

59c87d7

Merge remote-tracking branch 'origin/develop' into improved_file_reading

a7d1a59

Fix windows and linux bugs

2a3ed03

Fix bugs and warnings

ce64ba6

Merge remote-tracking branch 'origin/develop' into improved_file_reading

d324d33

Merge remote-tracking branch 'origin/develop' into improved_file_reading

f8d1a72

# Conflicts: # src/EnergyPlus/InputProcessing/InputProcessor.cc

Fix runtime error on AppleClang <12.0.5

4238639

Actually fix runtime error on AppleClang <12.0.5

0b17b5a

Fix bug in unit test

04234d4

Merge remote-tracking branch 'origin/develop' into improved_file_reading

9d1ad61

Myoldmopar approved these changes Sep 3, 2021

View reviewed changes

Myoldmopar merged commit 35e432b into develop Sep 3, 2021

Myoldmopar deleted the improved_file_reading branch September 3, 2021 13:48

mjwitte mentioned this pull request Sep 3, 2021

Crash when using ITE in an Uncontrolled Zone #9003

Merged

20 tasks

mbadams5 mentioned this pull request Sep 10, 2021

Fix bug when writing binary files #9054

Merged

20 tasks

jmarrec mentioned this pull request Sep 30, 2021

ExternalFile should enforce proper extension for E+ 9.6.0 NREL/OpenStudio#4451

Closed

jmythms mentioned this pull request Nov 1, 2021

Performance refactor for Schedule:File #8795

Closed

21 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve performance for file reading/writing #8996

Improve performance for file reading/writing #8996

mbadams5 commented Aug 23, 2021

Myoldmopar left a comment

Myoldmopar Sep 3, 2021

mbadams5 Sep 3, 2021

Myoldmopar Sep 3, 2021

mbadams5 Sep 3, 2021

Myoldmopar Sep 3, 2021

Myoldmopar Sep 3, 2021

Myoldmopar Sep 3, 2021

Myoldmopar Sep 3, 2021

Myoldmopar Sep 3, 2021

Myoldmopar Sep 3, 2021

Improve performance for file reading/writing #8996

Improve performance for file reading/writing #8996

Conversation

mbadams5 commented Aug 23, 2021

Pull request overview

Pull Request Author

Reviewer

Myoldmopar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment