But then you still have a custom string class in the API. The difference is that you can't modify it directly, only through conversions from other string types.
The idea was that it is only part of the public interface. StringConversion objects are never stored and would only exist as temporary objects during the duration of the call.
This has some drawbacks: when the StringConversion class is used in the API and std::basic_string<sf::Uint32> on user side, there would be a constant need to copy. An API of sf::String on the other hand allows the user to use sf::String as well, and pass const references.
Yeah... and what do you do with the const references? Right... you copy them into the object's own storage. This is the reason why it makes more sense to pass sink objects as values in C++11, because in almost all cases, you will be moving from an r-value, or performing a copy anyway. The only difference is
when you perform the copy. Browsing through SFML source, I couldn't find many places where the contents of the reference are merely "inspected" i.e. not copied. Almost all of them were in some form of conversion function back to wide strings, which implies copying yet again. Even if moves aren't present, copy elision would result in superior performance.
I took the time to create a test to measure the performance penalty of all these alleged copies that are potentially made.
#include <iostream>
#include <string>
#include <iterator>
#include <cstring>
#include <SFML/System.hpp>
class StringConverter {
public:
StringConverter( const char* ansiString, const std::locale& locale = std::locale() ) {
if( ansiString ) {
std::size_t length = strlen( ansiString );
if( length > 0 ) {
m_string.reserve( length );
sf::Utf32::fromAnsi( ansiString, ansiString + length, std::back_inserter( m_string ), locale );
}
}
}
StringConverter( std::basic_string<unsigned int> utf32String ) : m_string( utf32String ) {}
// More conversions here...
operator std::basic_string<unsigned int>() {
return m_string; // <-- Conversions return an r-value.
}
private:
std::basic_string<unsigned int> m_string;
};
class Text {
public:
void setString( StringConverter string ) {
m_string = string; // <-- Request a conversion from a (potential) r-value. Even if no move, copy can be elided.
// Just to prevent too much optimization.
if( m_string.empty() ) {
m_string += 1000;
std::cout << m_string.length();
}
}
private:
std::basic_string<unsigned int> m_string;
};
int main() {
std::basic_string<unsigned int> str( 100000000, 1000 );
for( int i = 0; i < 100000; i++ ) {
Text text;
text.setString( "Hello, World!" );
text.setString( str );
}
}
I built the executable 3 times using -O -O1 and -O2 with profiling enabled.
These are the flat profiles:
-O:
Flat profile:
Each sample counts as 0.01 seconds.
% cumulative self self total
time seconds seconds calls us/call us/call name
47.06 0.48 0.48 _mcount_private
14.71 0.63 0.15 _spin_lite_lock
12.75 0.76 0.13 _spin_lite_unlock
3.92 0.80 0.04 10300001 0.00 0.00 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_M_data() const
1.96 0.82 0.02 9000001 0.00 0.01 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_M_rep() const
1.96 0.84 0.02 2900001 0.01 0.01 __gnu_cxx::char_traits<unsigned int>::assign(unsigned int&, unsigned int const&)
1.96 0.86 0.02 1300000 0.02 0.02 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_M_is_shared() const
1.96 0.88 0.02 1300000 0.02 0.08 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::push_back(unsigned int)
1.96 0.90 0.02 600001 0.03 0.07 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::~basic_string()
1.96 0.92 0.02 300001 0.07 0.07 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_M_destroy(std::allocator<unsigned int> const&)
1.96 0.94 0.02 __pthread_self_lite
0.98 0.95 0.01 3300003 0.00 0.00 __gnu_cxx::new_allocator<unsigned int>::~new_allocator()
0.98 0.96 0.01 1300000 0.01 0.01 unsigned int sf::Utf<32u>::decodeAnsi<char>(char, std::locale const&)
0.98 0.97 0.01 600000 0.02 0.02 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_M_grab(std::allocator<unsigned int> const&, std::allocator<unsigned int> const&)
0.98 0.98 0.01 100000 0.10 0.17 Text::~Text()
0.98 0.99 0.01 _Unwind_SjLj_Unregister
0.98 1.00 0.01 __gnu_cxx::__atomic_add(int volatile*, int)
0.98 1.01 0.01 _fentry__
0.98 1.02 0.01 pthread_setspecific
0.00 1.02 0.00 4500003 0.00 0.00 __gcc_deregister_frame
0.00 1.02 0.00 3000000 0.00 0.01 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::size() const
0.00 1.02 0.00 2900002 0.00 0.00 __gnu_cxx::new_allocator<unsigned int>::new_allocator(__gnu_cxx::new_allocator<unsigned int> const&)
0.00 1.02 0.00 2700002 0.00 0.00 std::allocator<unsigned int>::~allocator()
0.00 1.02 0.00 2500003 0.00 0.00 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_M_refdata()
0.00 1.02 0.00 2300001 0.00 0.00 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::get_allocator() const
0.00 1.02 0.00 2300001 0.00 0.00 std::allocator<unsigned int>::allocator(std::allocator<unsigned int> const&)
0.00 1.02 0.00 1900002 0.00 0.00 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_M_set_sharable()
0.00 1.02 0.00 1600001 0.00 0.01 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_M_set_length_and_sharable(unsigned int)
0.00 1.02 0.00 1400000 0.00 0.01 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::capacity() const
0.00 1.02 0.00 1300000 0.00 0.08 std::back_insert_iterator<std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > >::operator=(unsigned int const&)
0.00 1.02 0.00 1300000 0.00 0.00 std::back_insert_iterator<std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > >::operator*()
0.00 1.02 0.00 1300000 0.00 0.00 std::back_insert_iterator<std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > >::operator++(int)
0.00 1.02 0.00 900001 0.00 0.02 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_M_dispose(std::allocator<unsigned int> const&)
0.00 1.02 0.00 600002 0.00 0.00 __gnu_cxx::new_allocator<char>::new_allocator()
0.00 1.02 0.00 600002 0.00 0.00 std::allocator<char>::allocator<unsigned int>(std::allocator<unsigned int> const&)
0.00 1.02 0.00 600001 0.00 0.00 std::allocator<unsigned int>::allocator(std::allocator<unsigned int> const&)
0.00 1.02 0.00 600001 0.00 0.00 std::allocator<unsigned int>::~allocator()
0.00 1.02 0.00 600001 0.00 0.00 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Alloc_hider::_Alloc_hider(unsigned int*, std::allocator<unsigned int> const&)
0.00 1.02 0.00 600001 0.00 0.00 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Alloc_hider::~_Alloc_hider()
0.00 1.02 0.00 600000 0.00 0.00 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_M_is_leaked() const
0.00 1.02 0.00 600000 0.00 0.00 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_M_refcopy()
0.00 1.02 0.00 600000 0.00 0.00 bool std::operator==<unsigned int>(std::allocator<unsigned int> const&, std::allocator<unsigned int> const&)
0.00 1.02 0.00 400001 0.00 0.00 __gnu_cxx::new_allocator<unsigned int>::new_allocator()
0.00 1.02 0.00 400001 0.00 0.00 std::allocator<unsigned int>::allocator()
0.00 1.02 0.00 400000 0.00 0.03 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::basic_string(std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > const&)
0.00 1.02 0.00 300001 0.00 0.00 __gnu_cxx::new_allocator<char>::deallocate(char*, unsigned int)
0.00 1.02 0.00 300001 0.00 0.00 __gnu_cxx::new_allocator<char>::allocate(unsigned int, void const*)
0.00 1.02 0.00 300001 0.00 0.00 __gnu_cxx::new_allocator<char>::max_size() const
0.00 1.02 0.00 300001 0.00 0.00 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_S_create(unsigned int, unsigned int, std::allocator<unsigned int> const&)
0.00 1.02 0.00 300001 0.00 0.00 operator new(unsigned int, void*)
0.00 1.02 0.00 300000 0.00 0.00 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_M_data(unsigned int*)
0.00 1.02 0.00 200001 0.00 0.01 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_S_construct(unsigned int, unsigned int, std::allocator<unsigned int> const&)
0.00 1.02 0.00 200000 0.00 0.07 StringConverter::~StringConverter()
0.00 1.02 0.00 200000 0.00 0.03 StringConverter::operator std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >()
0.00 1.02 0.00 200000 0.00 0.18 Text::setString(StringConverter)
0.00 1.02 0.00 200000 0.00 0.01 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::empty() const
0.00 1.02 0.00 200000 0.00 0.07 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::assign(std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > const&)
0.00 1.02 0.00 200000 0.00 0.01 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::basic_string()
0.00 1.02 0.00 200000 0.00 0.07 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::operator=(std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > const&)
0.00 1.02 0.00 100000 0.00 1.20 StringConverter::StringConverter(char const*, std::locale const&)
0.00 1.02 0.00 100000 0.00 0.03 StringConverter::StringConverter(std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >)
0.00 1.02 0.00 100000 0.00 1.13 std::back_insert_iterator<std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > > sf::Utf<32u>::fromAnsi<char const*, std::back_insert_iterator<std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > > >(char const*, char const*, std::back_insert_iterator<std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > >, std::locale const&)
0.00 1.02 0.00 100000 0.00 0.01 Text::Text()
0.00 1.02 0.00 100000 0.00 0.01 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_M_clone(std::allocator<unsigned int> const&, unsigned int)
0.00 1.02 0.00 100000 0.00 0.06 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::reserve(unsigned int)
0.00 1.02 0.00 100000 0.00 0.00 std::back_insert_iterator<std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > >::back_insert_iterator(std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >&)
0.00 1.02 0.00 100000 0.00 0.00 std::iterator<std::output_iterator_tag, void, void, void, void>::iterator()
0.00 1.02 0.00 100000 0.00 0.00 std::back_insert_iterator<std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > > std::back_inserter<std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > >(std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >&)
0.00 1.02 0.00 1 0.00 0.00 __gnu_cxx::char_traits<unsigned int>::assign(unsigned int*, unsigned int, unsigned int)
0.00 1.02 0.00 1 0.00 0.01 unsigned int* std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_S_construct<int>(int, int, std::allocator<unsigned int> const&)
0.00 1.02 0.00 1 0.00 0.01 unsigned int* std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_S_construct_aux<int>(int, int, std::allocator<unsigned int> const&, std::__true_type)
0.00 1.02 0.00 1 0.00 0.01 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_S_construct_aux_2(unsigned int, unsigned int, std::allocator<unsigned int> const&)
0.00 1.02 0.00 1 0.00 0.00 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_M_assign(unsigned int*, unsigned int, unsigned int)
0.00 1.02 0.00 1 0.00 0.01 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::basic_string<int>(int, int, std::allocator<unsigned int> const&)
0.00 1.02 0.00 1 0.00 0.00 std::_Iter_base<unsigned int*, false>::_S_base(unsigned int*)
0.00 1.02 0.00 1 0.00 0.00 __gnu_cxx::__enable_if<std::__is_scalar<unsigned int>::__value, unsigned int*>::__type std::__fill_n_a<unsigned int*, unsigned int, unsigned int>(unsigned int*, unsigned int, unsigned int const&)
0.00 1.02 0.00 1 0.00 0.00 std::_Niter_base<unsigned int*>::iterator_type std::__niter_base<unsigned int*>(unsigned int*)
0.00 1.02 0.00 1 0.00 0.00 unsigned int* std::fill_n<unsigned int*, unsigned int, unsigned int>(unsigned int*, unsigned int, unsigned int const&)
-O1:
Flat profile:
Each sample counts as 0.01 seconds.
% cumulative self self total
time seconds seconds calls ns/call ns/call name
37.50 0.03 0.03 main
12.50 0.04 0.01 400000 25.00 25.00 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::basic_string(std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > const&)
12.50 0.05 0.01 _Unwind_SjLj_Unregister
12.50 0.06 0.01 std::locale::~locale()
12.50 0.07 0.01 operator new(unsigned int)
12.50 0.08 0.01 _mcount_private
0.00 0.08 0.00 300001 0.00 0.00 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_S_create(unsigned int, unsigned int, std::allocator<unsigned int> const&)
0.00 0.08 0.00 200001 0.00 0.00 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_S_construct(unsigned int, unsigned int, std::allocator<unsigned int> const&)
0.00 0.08 0.00 200000 0.00 0.00 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::assign(std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> > const&)
0.00 0.08 0.00 100000 0.00 0.00 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_M_clone(std::allocator<unsigned int> const&, unsigned int)
0.00 0.08 0.00 100000 0.00 0.00 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::reserve(unsigned int)
0.00 0.08 0.00 1 0.00 0.00 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::_Rep::_M_dispose(std::allocator<unsigned int> const&)
-O2:
Flat profile:
Each sample counts as 0.01 seconds.
% cumulative self self total
time seconds seconds calls ns/call ns/call name
50.00 0.03 0.03 main
33.33 0.05 0.02 _spin_lite_lock
16.67 0.06 0.01 100000 100.00 100.00 std::basic_string<unsigned int, std::char_traits<unsigned int>, std::allocator<unsigned int> >::reserve(unsigned int)
0.00 0.06 0.00 400002 0.00 0.00 __gcc_deregister_frame
As you can see, with all optimizations disabled, your fear of excessive copies is prevalent. However with higher optimization levels, less and less is done which is a result of copy elision taking place in the possible places. This was not compiled with C++11 enabled and so moves shouldn't have been performed, if they would have been performed the compiler would optimize them away as well and the profile would have looked similar.
The idea is that because the C++ syntax doesn't allow us to express our intentions through direct conversions, we have to take the detour via temporary objects. However, because this is probably a very typical approach, compilers are already ready to optimize these scenarios away, perhaps to the point where the conversion takes place at the initial call site and copied directly into the target object's memory.
I know this is said a lot, but: Don't try to help the compiler, more often than not, it knows better than you what you are trying to do. And if you ask me, performance optimizations should really be a result of the implementation and not the public interface.
The word "allows" is important; it doesn't enforce it. It's already now possible to have std::basic_string<sf::Uint32> on user side, and implicitly convert it to sf::String.
The difference is that the user would have less of a headache working with the values they get back from SFML. Unless the conversion was implicit in the other direction as well, they will keep having to explicitly convert to std::basic_string<sf::Uint32> which isn't a very pretty sight.
At the moment we can't do much anyway: sf::String can't be removed, and a new solution should also take C++11 features into account, such as new standard character/string types and move semantics.
You didn't state the reasons why sf::String can't be removed, merely difficulties for which there is no optimal solution yet. You have to bare in mind that simply activating C++11 when compiling SFML will already make Tank's StringConversion proposal just as fast if not faster than the current implementation. Move semantics will be automatically incorporated into SFML classes (and the StringConversion class) thereby alleviating this constant fear of excessive copying where it is unnecessary. If execution time is dominated more by the conversion back to std::basic_string<sf::Uint32> in the user's code (which is a plausible scenario) then this solution already outperforms the current implementation even without C++11 enabled.
And regarding the new char and string types, it isn't as significant as some people might think. The only change is that the C++ standard finally provides definitions of the size of types that people can rely on. Before C++11 you would have to hope that whatever type you used would be stored using the same number of bits regardless of the platform the code was compiled for. This is pretty much what SFML tries to do with its sf::UintX and sf::IntX types, although with stronger (i.e. compiler conformance) guarantees. All you would have to do to make use of these "new" types is change the typedefs in Config.hpp to reflect the new types and std::basic_string<sf::Uint32> will be exactly the same as the typedefs provided by the standard. One must also not forget that perhaps std::u32string might not be what SFML wants to use, as there is no guarantee that char32_t is
exactly 32 bits wide, merely that it is
at least 32 bits wide which could hamper communication between hosts on different architectures.
I really can't see any real advantages for the user, performance-wise or development-wise by sticking with sf::String. As long as user code doesn't break (barring sf::String not being defined if simply removed instead of typedefed), it is a win-win scenario for everybody, even SFML maintainers as they would have to do less wheel-reinventing
.