View Issue Details

IDProjectCategoryView StatusLast Update
0001242OpenMPT[All Projects] Generalpublic2019-07-31 17:35
ReporterSaga MusixAssigned Tomanx 
Status assignedResolutionopen 
Product VersionOpenMPT 1.29.00.* (current testing) 
Target VersionOpenMPT 1.?? (long term goals)Fixed in Version 
Summary0001242: Eliminate unnecessary copies in charset conversion functions (std::string_view / no-ops)
  • Consider using std::string_view for charset conversion functions where it makes sense. This can save a copy in case the input to the function is not already a std::string. Might be useful e.g. for the scripting API and other interop code.
  • Use std::string rather than const std::string &in the parameters of in no-op string conversion functions. We can avoid a copy there in some (probably most) contexts.
TagsNo tags attached.
Has the bug occurred in previous versions?
Tested code revision (in case you know it)




2019-07-31 12:25

administrator   ~0003982

Using string_view before actually requiring C++17 results in either a ton of additional ifdef (which mptString.h and mptString.cpp already have plenty enough of anyway), or another zoo of additional string types named mpt::ustring_maybe_view for each and every string type that we have and still then result in strange const string_view & parameters (or again another set of ifdef). I would rather delay that until we require C++17.



2019-07-31 12:41

administrator   ~0003983

Using value parameters instead of const & for trivial iniline conversion functions pessimizes binary size for me: 4.447.744 bytes (before), 4.453.376 bytes (after) (VS2019 Win32 ReleaseShared).

Saga Musix

Saga Musix

2019-07-31 17:35

administrator   ~0003984

Let's wait for C++17 then.

Issue History

Date Modified Username Field Change
2019-07-30 19:18 Saga Musix New Issue
2019-07-31 12:08 manx Assigned To => manx
2019-07-31 12:08 manx Status new => assigned
2019-07-31 12:25 manx Note Added: 0003982
2019-07-31 12:41 manx Note Added: 0003983
2019-07-31 12:41 manx Product Version => OpenMPT 1.29.00.* (current testing)
2019-07-31 12:41 manx Target Version => OpenMPT 1.?? (long term goals)
2019-07-31 17:35 Saga Musix Note Added: 0003984