News:

Congratulations!
 You've won the News Item Lottery! Your prize? Reading this news item! :)

Modern C++?

Started by jamespetts, February 03, 2018, 12:39:25 AM

Previous topic - Next topic

0 Members and 2 Guests are viewing this topic.

jamespetts

I put this post in "patches and projects", although I am not sure that it is a perfect fit for this board as there is no patch and it is not a project as such - but I am not sure what other subforum would best suit this topic. Although I write from the perspective of developing the Extended branch, at least most of this will be equally applicable to Standard, which is why I have not posted this on the Extended subforum.

Simutrans is a rather old codebase. Much of the coding style in Simutrans (both Standard and Extended) dates from the 1990s when Simutrans was first written. Indeed, the project was started before even the C++98 standard was finalised, and finished only shortly afterwards.

Consequently, Simutrans has many characteristics of a codebase of its era, including copious use of raw pointers, char* or char[] strings, Simutrans-specific container classes and smart pointers (and minimal use of the STL), and little or no use of more recent language features such as inline initialisation, lambdas, constructor inheritance, the shared/unique/weak pointers and many others. Simutrans-Extended does use a C++11 feature, thread local storage, but that was a recent innovation necessary to make certain multi-threading aspects work effectively.

My own learning of C++ has been through the Simutrans codebase and what I was able to learn online when I first started working on Simutrans from the end of 2008, and I had largely missed learning about the new features introduced in C++11 and later and am only recently coming to realise that they mark a significant change in the way in which the language works, such that pre-C++11 coding styles and techniques are considered outdated. It is interesting to note that "modern C++" (as it seems that C++ using language features from C++11 onwards is now called) is enjoying a significant resurgence of interest after the importance of C++ as a language had appeared to wane in the 2000s, as the new language and STL features replicate many of the advantages of more modern languages whilst retaining the speed, cross-platform and backwards compatibility of C++.

To what extent is it worthwhile adopting some of these new language features in the Simutrans codebase (both Standard and Extended)? I do not imagine that adopting the new language features just for the sake of being up to date is likely to be worth the large effort involved in re-factoring myriad lines of code, but is there any merit in (1) selectively refactoring certain critical parts of the code; (2) adopting some or all conventions of modern C++ when code is refactored for other reasons; and (3) coding new features using some or all parts of the modern C++ style instead of the traditional C++ style?

I know that there has been some discussion of related issues in the past and the argument was made at the time that Simutrans needed to be able to compile on some very old platforms that had not even adopted compilers compatible with the C++98 standard. I do not know whether that is of non-trivial significance to any users of Simutrans-Standard now. It is definitely not relevant to Extended, which only aims to target Windows, Linux and Mac (and which since 2016 has required C++11 due to the aforementioned local storage), but I do not want to Extended codebase to diverge from the Standard codebase more than is necessary for Extended's different feature-set, so it is worthwhile adopting a joint approach to this issue where possible.

Does anyone have any idea whether the latest STL implementation for collection classes might be better or worse (for performance) than the Simutrans versions? I know that the Simutrans hashtable can be slow when there are many entries in it; but I do not know whether the STL version is any faster. Are the vectors better or worse? I notice that Simutrans has a weighted vector which is quite important that I do not believe that the STL has (although I could be wrong).

Simutrans even has its own min() and max() templates, which have caused real problems in the past: using them with 64-bit integers silently loses precision in ways that are not deterministic between different platforms, and I spent weeks tracking down network desyncs that were caused by this some time ago, eventually replacing the Simutrans min() and max() with std::min() and std::max() whenever 64-bit variables were used. Would it not be better to remove the Simutrans min() and max() entirely and use the STL versions instead to avoid this sort of problem in the future?

We will not get rid of anything but a fraction of Simutrans's raw pointers any time soon, but does anyone know how the homegrown quickstones compare to the new weak_ptr (together with shared_ptr and unique_ptr) in terms of performance? The latter are certainly more flexible in implementation, save that they cannot return a handle ID that can be used for purposes other than memory management as is currently done in many places with convoihandle_t and halthandle_t objects in particular. Is it worthwhile replacing NULL with nullptr? That could be done wholesale relatively quickly with a search/replace.

I wonder also whether using traditional C++ might over time increasingly deter newer developers from working on the project - but short of refactoring the codebase entirely, which seems impractical, I am not sure how one would handle that.

I should be very interested in thoughts from new and seasoned Simutrans developers alike on this topic.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

DrSuperGood

The reason Simutrans cannot use more advanced C++ revisions is because it has to support being built with compilers that existed long before many of the revisions were made.

Using the recent File Systems API of C++ would have solved all this unicode file path nonsense dead easilly, but instead one has to hack our own file system API. Like wise a lot of typing problems could be solved using feautres such as "auto" for local variables.

Seeing how Simutrans still must support only IPv4 and running off a single thread I do not expect this to change any time soon.

jamespetts

Do we have data on how many (if any) people are actually still using Simutrans on these old platforms in 2018?

Incidentally, I am in the process of experimenting with modern C++ in Extended by replacing the outdated, Extended specific ITERATE macro with the C++11 ranged for (currently in the vehicle-management branch for testing). In Extended, of course, this should not be an issue, since we need C++11 compatibility in any event for the thread_local keyword.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

DrSuperGood

As far as I am aware it is one of the Windows GCC ports that is the problem.

If C++ version is made, one should consider also performing the following...
  • Remove legacy Windows IPv4 code path. The Berkley socket implementation works on any reasonable version of Windows and supports both IPv4 and IPv6.
  • Force dependency on pThread. One cannot be multi-thread ignorant in this day and age where processors have more and more cores.
  • Migrate to using C++ FileSystem API if C++ version choosen supports it. Would solve a lot of platform specific file system problems.
  • Only support SDL2. Would clean up the 5 different front ends currently used down to 2.
  • Look to officially support 64bit builds. Although they currently work I am pretty sure there are optimizations to be had which could bring 64bit build performance in average use cases to parity or better than a x86 build.
  • Try to remove excessive Macro usage. A lot of the changes with recent C++ revisions were adding better template support so that one seldom has to use macros. Some of the loop macros are so complex that it breaks MSVC intellisense so it does not even recognize the holding function definition exists.

    The use of standard collection classes to replace the special Simutrans ones is something that would need to be profiled. The Simutrans ones were written such to be more efficient in certain aspects.

jamespetts

Those are all good suggestions. Is there any reason for Simutrans to be supporting outdated versions of Windows GCC ports? Msys can cope without difficulties with C++11, and supports all the versions of Windows that Microsoft still supports. Is there any reason for Simutrans to support older versions of Windows than Microsoft supports? Anyone with such very old hardware would probably be better off installing Linux in any event.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

prissi

Windows 7 (supported until 2020) is apparently not supported by MSVC 2017, only be previous versions (MSVC 2015). So this would be probably one limit.

The simutrans specific stuff is usually quite fast. For instnace the freelist, which was about 20-30 times faster than malloc/free (and had less overhead). I would suspect similar for the quickstone_tpl.

On most other things it is probably not so much different.

Also, according to this https://stackoverflow.com/questions/30829364/open-utf8-encoded-filename-in-c-windows teh filename issue is not fixed by C++ file-IO.

The question is, if potentially breaking code is worth the gain (of potentially finding errors or making the code base more appealing). Or if the time is not rather spend on something else.

jamespetts

The VS 2015 limitation might be a reason not to use C++17 features, but should not limit earlier features from C++11 and C++14. Indeed, I am using Visual Studio 2015 and can use thread local storage, the auto keyword, ranged for, etc.

For the collection classes, we might want to profile.

As to your final question, I suspect that the correct answer differs depending on the part of the code in question. Also, we need to consider whether to use new language features in new features or refactoring for other reasons.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Ters

Quote from: DrSuperGood on February 03, 2018, 01:28:49 AM
Using the recent File Systems API of C++ would have solved all this unicode file path nonsense dead easilly, but instead one has to hack our own file system API.

I'm not sure how that helps unless the endpoints are in on the game. Does libz, bzip2 and SDL support the new C++ API? Last I checked, they only had a C API. And that is only if the C++ API has been implemented correctly in the first place, and does not rely on the char C API being UTF-8.

Quote from: DrSuperGood on February 03, 2018, 03:00:30 AM

       
  • Remove legacy Windows IPv4 code path. The Berkley socket implementation works on any reasonable version of Windows and supports both IPv4 and IPv6.
What do you mean? I have not seen anything but Berkley sockets on Windows, in the form of Winsocks, and Winsocks supports both IPv4 and IPv6 through the same API (like Berkley sockets should).


Quote from: DrSuperGood on February 03, 2018, 03:00:30 AM

       
  • Force dependency on pThread. One cannot be multi-thread ignorant in this day and age where processors have more and more cores.

       
  • Only support SDL2. Would clean up the 5 different front ends currently used down to 2.
I really prefer having everything statically linked, as that mean I only have the executable and the settings.xml file to worry about when testing different versions, and only the former when upgrading. I don't have to get umpteen other DLLs onto the path. Multithreading the renderer is a lot of trouble that gives me nothing in return. If going modern is the goal, then moving rendering away from the CPU should be the goal, but as far as I have been able to figure out, that pretty much requires rewriting karte_t from scratch, including lots of changes in everything interacting with it. The dirty rectangle system has to operate on the world, not on the already rendered image.


Quote from: DrSuperGood on February 03, 2018, 03:00:30 AM

       
  • Look to officially support 64bit builds. Although they currently work I am pretty sure there are optimizations to be had which could bring 64bit build performance in average use cases to parity or better than a x86 build.
I don't think Simutrans can equally support both 32-bit and 64-bit without a lot of macro hacks. Even then, there is the issue that cache lines are the same size on both 32-bit and 64-bit x86. Increased pointer sizes might mean that you get bad cache line alignment, which in turn causes bad performance. However, I saw no indication of that back when I tried tried getting 64-bit builds to work at all. Maybe I didn't have big enough maps at the time, but I haven't seen people worrying about cache lines in a looong while, except here.


Quote from: DrSuperGood on February 03, 2018, 03:00:30 AM

       
  • Try to remove excessive Macro usage. A lot of the changes with recent C++ revisions were adding better template support so that one seldom has to use macros. Some of the loop macros are so complex that it breaks MSVC intellisense so it does not even recognize the holding function definition exists.
If code can't be properly inlined from a template by a compiler from the last 15 years, if probably can't be written with a macro without going insane. It would probably take some insane levels of nesting.


Quote from: prissi on February 03, 2018, 04:44:40 PM
The simutrans specific stuff is usually quite fast. For instnace the freelist, which was about 20-30 times faster than malloc/free (and had less overhead). I would suspect similar for the quickstone_tpl.


The C++ library is designed for plugable allocators, so we can use its container classes, but use freelist for the actual memory allocations. I'm don't think quickstone_tpl has a direct replacement in the C++ standard. One could perhaps solve the original problem with shared and weak pointers, but I don't know what their overhead is.


DrSuperGood

QuoteWindows 7 (supported until 2020) is apparently not supported by MSVC 2017
But... https://docs.microsoft.com/en-us/visualstudio/productinfo/vs2017-system-requirements-vs states that Windows 7 is supported. And...
https://docs.microsoft.com/en-us/visualstudio/productinfo/vs2017-compatibility-vs
States that building for Windows 7 is supported.
QuoteAlso, according to this https://stackoverflow.com/questions/30829364/open-utf8-encoded-filename-in-c-windows teh filename issue is not fixed by C++ file-IO.
That seems to be using boost, which would be the file system API before it was added as core to C++.
QuoteI don't think Simutrans can equally support both 32-bit and 64-bit without a lot of macro hacks. Even then, there is the issue that cache lines are the same size on both 32-bit and 64-bit x86. Increased pointer sizes might mean that you get bad cache line alignment, which in turn causes bad performance. However, I saw no indication of that back when I tried tried getting 64-bit builds to work at all. Maybe I didn't have big enough maps at the time, but I haven't seen people worrying about cache lines in a looong while, except here.
A lot of Simutrans relies of 64bit stuff to work however. So in the end it is all a balancing act to try and assure the negative performance from larger pointers is countered by the higher performance from more registers and 64bit instructions.
QuoteIf code can't be properly inlined from a template by a compiler from the last 15 years, if probably can't be written with a macro without going insane. It would probably take some insane levels of nesting.
Do not under estimate modern compilers, expect maybe MSVC...

Even if they cannot support it perfectly efficiently now, they might tomorrow.
QuoteWhat do you mean? I have not seen anything but Berkley sockets on Windows, in the form of Winsocks, and Winsocks supports both IPv4 and IPv6 through the same API (like Berkley sockets should).
This monster says otherwise...
#ifdef USE_IP4_ONLY
// Network load. Address format e.g.: "128.0.0.1:13353"
char address[1024];
static char err_str[256];
uint16 port = 13353;
const char *cp2 = strrchr(cp,':');
if(cp2!=NULL) {
port=atoi(cp2+1);
// Copy the address part
tstrncpy(address,cp,cp2-cp>sizeof(address)-1?sizeof(address)-1:cp2-cp+1);
cp = address;
}

struct sockaddr_in server_name;
memset(&server_name,0,sizeof(server_name));
server_name.sin_family=AF_INET;
#if USE_WINSOCK
bool ok = true;
server_name.sin_addr.s_addr = inet_addr(cp); // for windows we must first try to resolve the number
if((int)server_name.sin_addr.s_addr==-1) {// Bad address
ok = false;
struct hostent *theHost;
theHost = gethostbyname( cp ); // ... before resolving a name ...
if(theHost) {
server_name.sin_addr = *(struct in_addr *)theHost->h_addr_list[0];
ok = true;
}
}
if(!ok) {
#else
/* inet_anon does not work on BeOS; but since gethostbyname() can
* do this job on all other systems too, we use it only:
* instead of if(inet_aton(cp,&server_name.sin_addr)==0) { // Bad address
*/
struct hostent *theHost;
theHost = gethostbyname( cp );
if(theHost) {
server_name.sin_addr = *(struct in_addr *)theHost->h_addr_list[0];
}
else {// Bad address
#endif
sprintf( err_str, "Bad address %s", cp );
RET_ERR_STR;
}
server_name.sin_port=htons(port);

SOCKET my_client_socket = socket(AF_INET,SOCK_STREAM,0);
if(my_client_socket==INVALID_SOCKET) {
err = "Cannot create socket";
return INVALID_SOCKET;
}

if (connect(my_client_socket, (struct sockaddr*)&server_name, sizeof(server_name)) == -1) {
sprintf(err_str, "Could not connect to %s", cp);
RET_ERR_STR;
}

#else

Ters

Quote from: DrSuperGood on February 04, 2018, 01:31:11 AM
A lot of Simutrans relies of 64bit stuff to work however.

How can it? Which parts?

Quote from: DrSuperGood on February 04, 2018, 01:31:11 AM
Do not under estimate modern compilers, expect maybe MSVC...

Did you think I did? I was aiming for the opposite.

Quote from: DrSuperGood on February 04, 2018, 01:31:11 AM
This monster says otherwise...
#ifdef USE_IP4_ONLY
   // Network load. Address format e.g.: "128.0.0.1:13353"
   char address[1024];
   static char err_str[256];
   uint16 port = 13353;
   const char *cp2 = strrchr(cp,':');
   if(cp2!=NULL) {
      port=atoi(cp2+1);
      // Copy the address part
      tstrncpy(address,cp,cp2-cp>sizeof(address)-1?sizeof(address)-1:cp2-cp+1);
      cp = address;
   }

   struct sockaddr_in server_name;
   memset(&server_name,0,sizeof(server_name));
   server_name.sin_family=AF_INET;
#if USE_WINSOCK
   bool ok = true;
   server_name.sin_addr.s_addr = inet_addr(cp);   // for windows we must first try to resolve the number
   if((int)server_name.sin_addr.s_addr==-1) {// Bad address
      ok = false;
      struct hostent *theHost;
      theHost = gethostbyname( cp );   // ... before resolving a name ...
      if(theHost) {
         server_name.sin_addr = *(struct in_addr *)theHost->h_addr_list[0];
         ok = true;
      }
   }
   if(!ok) {
#else
   /* inet_anon does not work on BeOS; but since gethostbyname() can
    * do this job on all other systems too, we use it only:
    * instead of if(inet_aton(cp,&server_name.sin_addr)==0) { // Bad address
    */
   struct hostent *theHost;
   theHost = gethostbyname( cp );
   if(theHost) {
      server_name.sin_addr = *(struct in_addr *)theHost->h_addr_list[0];
   }
   else {// Bad address
#endif
      sprintf( err_str, "Bad address %s", cp );
      RET_ERR_STR;
   }
   server_name.sin_port=htons(port);

   SOCKET my_client_socket = socket(AF_INET,SOCK_STREAM,0);
   if(my_client_socket==INVALID_SOCKET) {
      err = "Cannot create socket";
      return INVALID_SOCKET;
   }

   if (connect(my_client_socket, (struct sockaddr*)&server_name, sizeof(server_name)) == -1) {
      sprintf(err_str, "Could not connect to %s", cp);
      RET_ERR_STR;
   }

#else


The Windows specific code seems unrelated to IPv4 vs. IPv6, although it only exists in the USE_IP4_ONLY case. Perhaps it has to do with incorrect/incomplete Berkley implementation in some ancient Windowses. USE_IP4_ONLY seems a bit of a misnomer in this case. It has more to do with legacy systems not supporting getaddrinfo. Maybe getaddrinfo was only introduced to Berkley sockets when IPv6 came along. I can't find any clear information about that. (It apparently came to Windows with IPv6 support.) Since getaddrinfo has been supported on Windows since XP (in Unicode since SP2), it can only be BeOS/Haiku, and a possible rule to still support Windows 2000, that requires the USE_IP4_ONLY code. I think it is five years or more since we raised minimum system requirements to Windows 2000. We still had vocal users using Windows 2000 at the time, but is that system still in use for Simutrans? I have no idea about BeOS/Haiku.

prissi

The Haiku user numbers are single digit (15 month accumulated), and AmigaOS slightly more (in then tens).

Also the code was from a time, when winsock2 was not default. Anyway, if it decreases performance and maintanbility, then lets take it out.

About putting time and effort into changing existing code to new C++ feature: I would only do this if there is much to gain.

The freelist overloads new, so I do not think hooking up later in the caller chain can ever give any additional gain. Also it is already very much proper C++. Not every new feature is more perfomant, on the contrary, many new concepts are complex and requires more logic for compiler to properly optimise. Most performant would be pure C code without class overhead ... the C++ is rather for maintainabilty (or how this is spelled) and confort, at least this was my impression.

Dwachs

Thanks for bringing this up. However, I fear the discussion will not lead to anything.

What new C++ features would help us to improve the code base?
Parsley, sage, rosemary, and maggikraut.

jamespetts

Quote from: Dwachs on February 04, 2018, 01:28:14 PM
Thanks for bringing this up. However, I fear the discussion will not lead to anything.

What new C++ features would help us to improve the code base?

That needs careful thought. What is worth doing for new features is a different question to what is worth the effort of re-factoring existing features that work well, and that is different again to what is worth the effort for re-factoring code that is found not to work well.

In the first category, the auto keyword and ranged for make things much easier for the coder. It is possible that more reliable code can be produced in many cases by using the new smart pointer types instead of raw pointers (but I suspect that it is not likely to be worthwhile replacing the existing quickstones with them).

I can see that move semantics might be quite useful in one or two places (e.g. when copying/moving schedules in some instances, perhaps).

Given the problems with the built-in min/max functions, I should recommend removing those and replacing them with std::min() and std::max().

As to the collection classes, the Simutrans specific ones seem reliable, but if the STL ones perform non-trivially better, it would probably be worth using those instead (but only profiling will determine this).

An open question is the extent to which having modern coding alongside traditional coding for an extended period of time will make the code less readable.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Ters

Quote from: prissi on February 04, 2018, 12:36:00 PM
Most performant would be pure C code without class overhead ... the C++ is rather for maintainabilty (or how this is spelled) and confort, at least this was my impression.

Not quite. Non-polymorphic C++ classes should have the same performance as a plain C struct. If you want polymorphism, then I think you might be able to do it in C with less indirect jumps than the typical C++ implementation, but that requires more memory and more instructions when allocating new objects (by inlining the vtable in each object). Of course, anything that can be done in C can be done in C++.

An std::vector should be as fast as a plain dynamically allocated array, as long as you stick to the unchecked accessors. C++ might make it easier to do things inefficient, though, as some details might be hidden. I have no idea how many allocations take place in a = b + c + d when all four variables are std::strings.

Quote from: Dwachs on February 04, 2018, 01:28:14 PM
What new C++ features would help us to improve the code base?

The auto keyword can save some typing with complicated template types, in particular iterators. Simutrans uses some macro magic to hide these types, but those macros are something new developers have to figure out what does, rather than recognizing a standard C++ construct. Using it for primitives can however introduce bugs as it is no longer clear whether an integer variable is signed or how big it is.

There are now standard implementation of hash maps and single-linked lists in C++, with an interface C++ developers should be familiar with. It has to be seen how they perform, though, and if they fit what Simutrans does. There might be ways of doing a single-linked list faster than the C++ standard, by making a less encapsulated implementation where the data structure being stored is an active participant, rather than ignorant about being stored in such a list.

Using templates for things like min and max might be more typesafe, but that has nothing to do with modern C++. The necessary template functionality has been there since day one, I think.

jamespetts

Quote from: Ters on February 04, 2018, 06:32:22 PM
Using templates for things like min and max might be more typesafe, but that has nothing to do with modern C++. The necessary template functionality has been there since day one, I think.

If one uses std::min() and std::max(), surely there is no need to use templates?
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Ters

Quote from: jamespetts on February 04, 2018, 06:56:49 PM
If one uses std::min() and std::max(), surely there is no need to use templates?

They are templates.

DrSuperGood

QuoteHow can it? Which parts?
Since floating points cannot be used, there is a lot of fixed point hackery going on. To prevent overflows these calculations are done using 64bit primitives. Only x86-64 added instructions to perform operations on 64bit primitives. In x86 the primitives have to be manipulated with a sequence of instructions (slower). Additionally the extra registers of x86-64 would speed up the calculations by holding intermediate results instead of such values having to be written out to stack.

Additionally a lot of values are stored in 64bit types for the extended number range. Examples include city growth, currency, sum totals for monthly averages, etc.

The factory code is full of 64bit types being operated on. I also beleive the convoy movement logic is a similar case. Power also uses a lot of 64bit types.

QuoteThe Windows specific code seems unrelated to IPv4 vs. IPv6, although it only exists in the USE_IP4_ONLY case. Perhaps it has to do with incorrect/incomplete Berkley implementation in some ancient Windowses. USE_IP4_ONLY seems a bit of a misnomer in this case. It has more to do with legacy systems not supporting getaddrinfo. Maybe getaddrinfo was only introduced to Berkley sockets when IPv6 came along. I can't find any clear information about that. (It apparently came to Windows with IPv6 support.) Since getaddrinfo has been supported on Windows since XP (in Unicode since SP2), it can only be BeOS/Haiku, and a possible rule to still support Windows 2000, that requires the USE_IP4_ONLY code. I think it is five years or more since we raised minimum system requirements to Windows 2000. We still had vocal users using Windows 2000 at the time, but is that system still in use for Simutrans? I have no idea about BeOS/Haiku.
Seeing how more and more ISPs only offer IPv6 addresses to their customers should one even support the option of IPv4 only? As far as I am aware that code is as good as unmaintainable. Even if it has bugs chances are no one uses it to report them.

I thought minimum system requirements were Windows XP.

QuoteWhat new C++ features would help us to improve the code base?
The "auto" keyword for local variables. Would solve a lot of type warnings, especially with loops.

The filesystem library might be the holy grail solution to the Unicode file name nonsense. It might also just be another name for the same nonsense. Would have to be tested.

There are also several useful localization related features that were added. For example printing time or numbers in a locale friendly way without having to reinvent the wheel and ending up making it an oval or square.

jamespetts

Quote from: Ters on February 04, 2018, 08:55:10 PM
They are templates.

Interesting - they are not the normal templates requiring <>s, so they must have some very interesting way of deducing the type automatically. Either way, they work properly with 64-bit integers whereas the built-in Simutrans ones do not.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

TurfIt

Actually they do require <>s as the 'interesting' auto stuff fails. Attached a patch I started the second time Simutrans own sint32 only min max burned me, but still needs verification....
It's not only 64 bits types that silent conversions to sint32 fail - max value uint32s don't work either!

IMHO nothing should ever be 'auto'. Automatic things always fail in strange ways - make it explicit and one always knows what's going on.



DrSuperGood

QuoteIMHO nothing should ever be 'auto'. Automatic things always fail in strange ways - make it explicit and one always knows what's going on.
This is only needed with the case where mathematics/logic is important. This is because one has to explicitly state what is going on. For example the fixed point hackery used by industry, power and convoy movement logic.

If one is manipulating an object the type is pretty much implicit so all not using auto is doing is repeating what the person already knows.

SomeType* obj = new SomeType(); // It was assigned new SomeType so it is obviously SomeType*...
auto obj = new SomeType(); // obj is clearly SomeType* because the assignment explains it all.

If one were to change the type of object used only 1 change would be needed with auto, as opposed to 2 where both object and type has to change. Type safety still applies, the compiler will throw a compile time error or warning if one tries to use the variable as a different type of argument.

There is also an explicit nullptr value, as opposed to a macro or 0.

Ters

Quote from: DrSuperGood on February 04, 2018, 09:52:23 PM
Since floating points cannot be used, there is a lot of fixed point hackery going on. To prevent overflows these calculations are done using 64bit primitives. Only x86-64 added instructions to perform operations on 64bit primitives. In x86 the primitives have to be manipulated with a sequence of instructions (slower). Additionally the extra registers of x86-64 would speed up the calculations by holding intermediate results instead of such values having to be written out to stack.

Additionally a lot of values are stored in 64bit types for the extended number range. Examples include city growth, currency, sum totals for monthly averages, etc.

The factory code is full of 64bit types being operated on. I also beleive the convoy movement logic is a similar case. Power also uses a lot of 64bit types.

You don't have to build a 64-bit application to use the 64-bit registers and instructions. All it takes is raising the minimum targeted CPU to one that supports them. Whether it then makes any sense not to go full 64-bit is a different question.

Quote from: DrSuperGood on February 04, 2018, 09:52:23 PM
The "auto" keyword for local variables. Would solve a lot of type warnings, especially with loops.

My point is that it might simply propagate the error rather than raise a warning, because the loop variable was of the right explicit type, but the input was wrong. With auto, it would simply default to the wrong type from the inputs.

Quote from: jamespetts on February 04, 2018, 10:19:04 PM
Interesting - they are not the normal templates requiring <>s, so they must have some very interesting way of deducing the type automatically.

They use <> in the declarations (and definitions). Only when using templated types to you normally need to use <> when using them. For function calls, the templated parameter types can usually be deduced from the arguments (and perhaps what type you want to store the returned value in).

DrSuperGood

QuoteYou don't have to build a 64-bit application to use the 64-bit registers and instructions. All it takes is raising the minimum targeted CPU to one that supports them. Whether it then makes any sense not to go full 64-bit is a different question.
As far as I am aware one does. The 64bit instructions can only be executed if the processor is running in 64bit mode which it will only do so when running a 64bit application. When running 32bit applications it runs in 32bit mode which does not support those instructions and registers. This is how backwards compatability was achieved, via some use of special registers/addresses to change CPU mode.

Exception are the vector instructions. However these are intended to manipulate multiple 8, 16, 32, or 64 bit values simultaniously and not for individual calculations. For simple 64bit operations on a single value they are likely considerably slower than using a single x86-64 instruction. At the very least they have 2 instruction overhead to use as one has to explicitly state they are in use and no longer in use so that OS context switching knows to dump their values.

Ters

My memory must have been playing tricks on me. I was certain I had seen the compiler spit out 64-bit instructions in a 32-bit compile. 32-bit registers are also available in 16-bit mode. But I can't reproduce it, and the manual seems indeed very explicit that it can't be done. (Were they really out of possible prefixes?)

TurfIt

Thinking of the x32 ABI perhaps? Such would be ideal for Simutrans, and any program that can't effectively use more the 4GB. All the goodness of x86_64 registers/instructions but still 32 bit pointers for compactness. Shame not widely supported...

Ters

Well, that reminds me of what I was thinking of, and it is possible it was on Linux I saw what I thought I saw. On the other hand, this seems to be 64-bit applications that has an agreement with the OS to only use 4GB of address space. That is a strange quirk I probably would have remembered even better.

DrSuperGood

Quote(Were they really out of possible prefixes?)
Using prefixes for it is counter productive. The entire point of x86-64 was to try and adopt a RISC like design. By using suffixes and also supporting all of x86 at the same time it would become just another CISC extension adding to the complexity of the system.

Only recently have people considered going back to CISC for some features as it can be faster thanks to pipeline optimizations and micro code. For example a CISC memory copy instruction with repeat suffix can be generally the most efficient way to perform bulk memory copies on modern processors as micro code will automatically convert it to the most optimized memory copy routine. On the other hand the numerous general purpose registers of RISC design is here to stay, and one of the big performance advantages of using x86-64 over x86.

Ters

Quote from: DrSuperGood on February 05, 2018, 10:27:05 PM
Using prefixes for it is counter productive. The entire point of x86-64 was to try and adopt a RISC like design.

One would think so, but according to the Intel instruction reference I have, x86-64 uses prefixes to access the new general purpose registers. I get mixed signals whether a prefix is needed to access 64-bit general purpose registers at all.

prissi

While this slowly gets into 32 bit versus 64 bit. Simutrans need to supply 32 bit builds for windows for the next five years or so, since at least until last year low end hardware was sold with Windows in 32 bit mode on atoms. SImutrans currently runs fine on those.

About auto keyword: Are there really so many place where it would save typing? Again, considering the effeort of changing versus the time better spend on other stuff ...

IP4 was the only platform supported by the Amiga builds, Not sure, if there is a user still around.

Ters

Quote from: prissi on February 07, 2018, 05:48:02 AM
About auto keyword: Are there really so many place where it would save typing? Again, considering the effeort of changing versus the time better spend on other stuff ...

I don't think we should make an effort to actively change things just because we can. It would be only for newly written or rewritten code. Places were it really would save on typing would be:


for (auto iter = vehicles.begin(); iter != vehicles.end(); ++iter) {
  ...
}

vs

for (std::vector<road_vehicle_t>::iterator iter = vehicles.begin(); iter != vehicles.end(); ++iter) {
  ...
}


and, although I'm not sure Simutrans does this


auto result = map.insert(std::make_pair(1,"Hello"));

vs

std::pair<std::map<int,std::string>::iterator,bool> result = map.insert(std::make_pair(1,"Hello"));


One can make the worst cases shorter with using namespace std;, but still.

But I would like to have the type given explicitly in

for (auto i = some_var; i < get_something(); ++i) {
  ...
}

rather than having to look up what type some_var is and get_something returns and then figure out what the compiler should deduce auto to be. If i is explicitly typed, and this seems add odds with some_var and/or get_something(), then at least GCC 5+ issues warnings.

jamespetts

Indeed - auto is not really something worth going back and changing retrospectively, but can be useful for new code. Also,


for (auto iter = vehicles.begin(); iter != vehicles.end(); ++iter) {
  ...
}


can actually become


for (auto iter : vehicles) {
  ...
}


in C++11, and I have confirmed that it works without difficulties using the Simutrans collection classes rather than the STD/STL collection classes.

One thing that might be worthwhile going and changing retrospectively (because it is extremely easy with search and replace) is NULL to nullptr. If we are able to use C++11 language constructs such as auto and ranged for generally, then it seems to me that this makes sense. Likewise, the built-in Simutrans min() and max() templates are problematic as discussed, and these can very easily be changed to std::min() and std::max() with search and replace.

It may be best to think of this in two distinct stages:

(1) does the Simutrans codebase need to retain compatibility with compilers that are incapable of dealing with C++11 code; and

(2) if not, what new C++11 language features are worth deploying in any given case of new/existing code?

If we are answering question (1) in the negative, then things such as nullptr > NULL and std::min/max() over Simutrans min/max() are very straightforward. Other things, such as STD/STL collection classes, STD smart pointers, the auto keyword, would need to be considered on a case by case basis whenever code is refactored. We may want to consider whether to refactor the FOR macros which are non-standard and difficult to read for those not familiar with them to the ranged for construct, which is much easier to read (the auto keyword is not compulsory). That refactoring should not be too difficult in principle.

If, conversely, we are answering the first question in the affermative, then all of this is moot in any event, although that would be unfortunate, I think.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

prissi

The FOR macros are working and are just another way of writing the auto ranged for. They have the advantage of telling the type explicitely, so I like them more than the ranged for. Also I am not sure how the ranged for reacts to a change of end() during iteration, which the macro allows. Mixing iterators is not good (as we would have then three different styles in the code). So yet another style, hmm.

If the min/max from std:: behave the same as the existing templates (in terms of unsigned versus signed), yes why not. Will also remove some min max macro trouble from windows includes.

About C11++ compilers, GCC5 is supported by Haiku, GCC 6.x by AmigaOS, so there should be no problems even with those exotic platforms.

Ters

Quote from: prissi on February 07, 2018, 02:29:23 PM
The FOR macros are working and are just another way of writing the auto ranged for. They have the advantage of telling the type explicitely, so I like them more than the ranged for. Also I am not sure how the ranged for reacts to a change of end() during iteration, which the macro allows. Mixing iterators is not good (as we would have then three different styles in the code). So yet another style, hmm.

I think that is the problem. For new Simutrans developers, the FOR macro is another Simutrans specific thing they have to learn what does and how to use, whereas plain C++ for loops are either known from before or useful knowledge to have beyond Simutrans. I seem to take such oddities and quirks with ease, but I notice that not all developers do so.

jamespetts

Splendid, it is useful to have a good idea as to the progression of this matter.

May I ask whether a view has been taken on nullptr/NULL?
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

DrSuperGood

QuoteMay I ask whether a view has been taken on nullptr/NULL?
nullptr is better as it is a strongly typed constant where as NULL is a macro to a numeric constant. Macros cause abstraction of values because from compiler error output it can be hard to find the problem with a macro.

Like wise using the appropriate types for pointer difference operations is recommended over size_t, or worse int.

prissi

nullptr is indeed the best choice, I would agree with that too. Again, needs to be all replaced, and not mixed.