News:

Use the "Forum Search"
It may help you to find anything in the forum ;).

Incorporating changes from Standard

Started by ACarlotti, May 06, 2018, 12:08:01 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

jamespetts

Quote from: Ranran on November 24, 2020, 02:35:58 PM
Changes from recently added standards:
The estimate cost is now displayed when creating a tunnel. This is useful because you know where you can build the (straight) tunnel.
However, the calculation is different from standard and is currently incorrect.
It seems that it is not just the sum of the total amount of the tunnel and the way. I would be grateful if you could help me with this.

In Extended, the cost of building a tunnel is: (1) the cost of tunnelling (represented as the cost of the tunnel object); (2) the cost of the way; (3) the forge cost (only if building a new tunnel, not upgrading an existing tunnel, and halved if building parallel with an existing tunnel); and (4) the cost of the wayleave (1/5th of the cost of the land unless the player already owns the land above).

Quote
Considering the existence of color buttons such as profit, white may be desirable for general buttons. (It turns light blue when pressed.)

White would fit the theme better than orange, but would it not be clearer for them to be blue when not pressed and white when pressed?

Quote
This I explained before. This dialog hasn't been updated yet, and its overlap is caused by a special code created by Bernd Gabriel.
The update for this dialog didn't fit the tab well because I'm not familiar with the timetable code.
I pushed the branch I tried to merge. Can you confirm this? I'm stumbling on the control tower process.
https://github.com/Ranran-the-JuicyPork/simutrans-extended/tree/std-haltinfo

After it works properly, I need to adjust the dialog to the extended spec.

My apologies: I had forgotten that this had already been dealt with: there are a great many things that have been discussed so far in this thread, so it is not difficult to lose track of one of them.

I have tested this branch, but, unfortunately, the halt information dialogue does not work: I get an infinitely repeating assertion failure at line 33 of gui_image.cc.

Quote
I've broadly grouped commits by feature to get a rough view, and I probably know the commits about it. But it may not be an easy task, so I skipped it. I'm working a little now.

That is extremely helpful, as this has a potential to make a big difference to the online games. There is not any fundamental difference in the compression code between Standard and Extended, so I anticipate that the most difficult aspect of this will be setting up the necessary libraries.
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.

Ranran

#211
QuoteExcellent - thank you for this. This does indeed appear to work (although it also seems to override some theme settings, e.g., the title bar colour in the "modern" theme reverts to the orange of new/classic; is that intended?)
Can you elaborate on this?

As for the cursor color, it seems to inherit the value of the previous theme if theme doesn't specify it. This behavior is the same as standard.


Quotebut would it not be clearer for them to be blue when not pressed and white when pressed?
The text color of the pressed button is the same for the colored button and the normal button. So for modern themes, the text color of the pressed button is all white.
Based on prissi's advice, it seems possible to lighten the background color of the pressed color button, in which case we need to swap the color of the text button and background color of the color button image.


QuoteI have tested this branch, but, unfortunately, the halt information dialogue does not work: I get an infinitely repeating assertion failure at line 33 of gui_image.cc.
The departure board code is very different from the standard and it is difficult for me to port it correctly. Therefore it remains broken. It is possible to open a station where convoy does not arrive and depart, but if you run convoy with the dialog open, it will crash.
In the new GUI, GUI parts must be contained in components.

EDIT:
I noticed that halt info was broken at the time of the standard's first commit. The issue of crashing when picking up a commit that fixes it has been fixed.
So the current problem is that I can't change the departure board from the standard form to the extended form.

Ranran

#212
The work of incorporating zstd has advanced and stucked. Σ(´・ω・`)  [ Stuck ]
You can sample this in here.
https://github.com/Ranran-the-JuicyPork/simutrans-extended/tree/std-zstd
All build attempts on guihub seem to be able to build, but MSVC fails to build on my PC. This may be the reason why I've been knocked down by standard builds so far.
We need to prepare a new file related to zstd. So I have tried many versions from 1.3.3 to 1.4.5 but all have failed.

1>loadsave.obj : error LNK2019: 未解決の外部シンボル _ZSTD_compressStream2 が関数 "private: void __thiscall loadsave_t::flush_buffer(int)" (?flush_buffer@loadsave_t@@AAEXH@Z) で参照されました。
1>libzstd_static.lib(fse_decompress.o) : error LNK2001: 外部シンボル "___chkstk_ms" は未解決です。
1>libzstd_static.lib(zstd_v04.o) : error LNK2001: 外部シンボル "___chkstk_ms" は未解決です。
1>libzstd_static.lib(zstd_v07.o) : error LNK2001: 外部シンボル "___chkstk_ms" は未解決です。
1>libzstd_static.lib(zstd_v05.o) : error LNK2001: 外部シンボル "___chkstk_ms" は未解決です。
1>libzstd_static.lib(zstd_v06.o) : error LNK2001: 外部シンボル "___chkstk_ms" は未解決です。
1>libzstd_static.lib(huf_compress.o) : error LNK2001: 外部シンボル "___chkstk_ms" は未解決です。
1>libzstd_static.lib(fse_compress.o) : error LNK2001: 外部シンボル "___chkstk_ms" は未解決です。
1>libzstd_static.lib(hist.o) : error LNK2001: 外部シンボル "___chkstk_ms" は未解決です。
1>libzstd_static.lib(huf_decompress.o) : error LNK2001: 外部シンボル "___chkstk_ms" は未解決です。
1>libzstd_static.lib(fse_compress.o) : error LNK2019: 未解決の外部シンボル ___udivdi3 が関数 _FSE_normalizeCount で参照されました。
1>.\simutrans\Simutrans-Extended-debug.exe : fatal error LNK1120: 10 件の未解決の外部参照


Did I fail to set up the project file? (´・ω・`)はぁまじ

I would appreciate any help in this regard.

EDIT:
__imp__ was not seen when building debug builds.

jamespetts

Thank you for your work on this.

The __imp__ prefixes in the linker errors suggest that the zstd library that you have compiled has been compiled to be dynamically rather than statically linked. Was that what you intended? If not, this is a possible source of at least part of this error.
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

This is a common problem. The zlib library was compiled using the dynamic runtime library. The easiest fix is to compile zstd lib by hand using static linking. Becasue the rtl is different between different MSVC version, it is most easy to compile it with the version you are using.

Ranran

#215
@prissi
Thank you for your accurate advice.  :hat:
I downloaded the 1.4.5 source from the zstd official website, compiled the lib and dll myself, and used it, and it built correctly.   :)


Now you can test the zstd branch for extended.
In my verification work, the save size is about 30% smaller than zipped, and the zstd option seems to be working properly.

EDIT:
Save and setting file is not compatible with the r8653-2 branch because I have not changed the revision number.
The sound settings dialog has been updated in zstd branch. This is because they are implemented in parallel in standard and I have incorporated them together to avoid conflicts and reduce the overall workload.

EDIT2:
From now on, what the zstd branch should update is the content of <a href="https://forum.simutrans.com/index.php/topic,20488.0.html">this thread</a> (r9406 and later). But that's the latest change that has happened in the last week. The opinions of experts on what to do about this are helpful.

jamespetts

#216
Thank you very much for your work on this. I have now managed to get the zstd branch working. Two points in relation to that: first of all, the saved game format is not compatible with the latest master branch saved game format, as this has incremented recently; you may need to look into this. Secondly, this merges the new Standard sound distance/volume system, which I suspect clashes with code with a similar intention already present in Extended; it may be necessary either to remove the Extended implementation or the new Standard implementation, whichever is the less satisfactory of the two (I have not tested extensively yet).

As to zstd itself, at compression level 6, using the Bridgewater-Brunel saved game from 1940, loading using the existing Zlib took 33 seconds, whereas loading with the zstd took 1:23 minutes. Saving with zstd took 4:23 minutes. At compression level 3, that was only modestly better at 3:16 minutes. Zlib saved the same game in 35 seconds at compression level 3. The saved game size with zstd compression level 3 was 396MB, whereas for zlib compression level 3, it was 474MB. At compression level 6, zlib took 1:13 minutes to save this game, and produced a file 429MB in size. The file being loaded from the Bridgewater-Brunel server and then immediately re-saved was 484MB, suggesting an even lower compression level used there.

These results are disappointing and confusing (although I do not blame Ranran for this, who has very helpfully ported the Standard implementation of this): TurfIt's tests last year suggested the possibility for a very significant speed improvement in using zstd over zlib, but current tests show this not to be possible, although instead suggesting that reducing the compression level may help significantly (and the ability to adjust the compression level is also new in this code, so this work will still be worthwhile even if the zstd compression is not used in the end).

If anyone who is able to give any insight into why the improvements demonstrated by TurfIt last year may not have been realised in this implementation, that would be very helpful.

In the meantime, it would be helpful if Ranran could look into the two smaller issues set out above. I will have to investigate setting up the build server to have the right libraries to work with zstd to make sure that this version can build.

I should note that I should very much like to be able to get these changes merged into the master branch by the 19th of December this year if possible, and I think that we are getting very close with the UI work.

Thank you again to Ranran for all the work in this regard.
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.

Ranran

Quotefirst of all, the saved game format is not compatible with the latest master branch saved game format, as this has incremented recently; you may need to look into this.
Certainly it causes problems as I missed that the revision of the master branch was incremented.
Is it okay to change the version number to 15.0 because the amount of integration from the standard is huge?


QuoteSecondly, this merges the new Standard sound distance/volume system, which I suspect clashes with code with a similar intention already present in Extended; it may be necessary either to remove the Extended implementation or the new Standard implementation, whichever is the less satisfactory of the two (I have not tested extensively yet).
I haven't looked in detail at whether its function has been broken or if it can coexist.
But at least the new standard feature, which allows player to adjust the volume of the sound individually, seems like a big improvement.


Quoteinstead suggesting that reducing the compression level may help significantly
I tried saving with a large map in standard and I think this is correct. The low compression zipped is much faster than the high compression zstd.

jamespetts

Quote from: Ranran on November 30, 2020, 10:12:10 PM
Certainly it causes problems as I missed that the revision of the master branch was incremented.
Is it okay to change the version number to 15.0 because the amount of integration from the standard is huge?

No, this is probably not sensible, as 15 is reserved for the vehicle-management branch, integration and completion of which is the next major step after integrating the work on this merging.

QuoteI haven't looked in detail at whether its function has been broken or if it can coexist.
But at least the new standard feature, which allows player to adjust the volume of the sound individually, seems like a big improvement.

Yes, the ability to alter the individual levels is a worthwhile thing indeed; it may well be that Standard's sound distance system may be worth adopting, but this will need further testing.

QuoteI tried saving with a large map in standard and I think this is correct. The low compression zipped is much faster than the high compression zstd.

Thank you for confirming. It is odd that this is so different to the results of TurfIt's tests last year.
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

Of course high compression zstd is much slower than low compression zipped.  3 times slower for a 30% smaller file per January's tests.
But at default compression, both produce the same size file with zstd 10 times faster, or zstd 2 times faster than the fastest zip setting.
However that was using the wrapper. Would appear the non-wrapper implementation is non-performant.

Ranran

Extended zstd branch may be making some difference as the progress bar is not smooth.
Uncompressed zstd with seve_level = -10 is slower than zipped with save_level = 1....

TurfIt

Quick test with standard:  zstd wrapper performs identical to zstd non-wrapper (prissi's implementation). Not a factor.
zstd level 6 compresses in 1/3 the time of zlib level 6 producing identical sized file. About expected.

SimEx remains unlinkable so can't test anything with it...

prissi

zstd gained very little head in most test games in standard, unless using very high levels (and thus being slow). Hence it was not set a default server format. I would use zstd (and then with very high settings) only when transfer speed becomes an issue, either on ADSL hosting or indeed with GB sized games.

TurfIt

I've never seen a large enough game that Standard has any issue with saving in a reasonable time  (unless using bzip2 - that's slow!).
zstd makes a noticeable improvement percentage wise, but the absolute time is so quick it doesn't really matter.  Eg. Save the yoshi test game - zlib 680 ms, zstd 250 ms. Both level 6 yielding same filesize.
EDIT: tried an old game 'dom28.sve'. No idea where from.... zlib 8400 ms, zstd 2250 ms. Getting worth it...

prissi

But zlib at level 3 is way way faster than zstd with files about 10% larger. And bzip2 was not much slower than zstd in my tests. Apparently it strongly depends on the actual zlib lib one was using ... (I had to use own compiled zlib for MSVC, maybe that is the reason.)

jamespetts

Ranran and TurfIt - thank you for testing: that is helpful. TurfIt - what linking problems are you having with Extended?

It remains unclear why TurfIt's tests from last year seem inconsistent with the results that we are seeing now in Extended. However, one issue is that I am not sure how the compression levels compare. Is zstd at level 3 equivalent to zlib at level 3 or is there some conversion to be applied? Note that my tests above were using level 3 on both, as reported (aside from the instances where I used the default level of 6, also noted).
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.

Ranran

# zip and zstd allow for finetuning their packaging versus speed with and additional
# compression level parameter.
# Zip form 1(fastest) to 9(smallest) with 6 a good compromise
# zstd goes form -10 to 30 or so. Meaningful are mostly single digit values
save_level = 6
autosave_level = 1

I don't think the same setting value for different compression formats has the same compression ratio.

jamespetts

Quote from: Ranran on December 01, 2020, 12:40:51 PM
# zip and zstd allow for finetuning their packaging versus speed with and additional
# compression level parameter.
# Zip form 1(fastest) to 9(smallest) with 6 a good compromise
# zstd goes form -10 to 30 or so. Meaningful are mostly single digit values
save_level = 6
autosave_level = 1

I don't think the same setting value for different compression formats has the same compression ratio.

Yes, this is an inference from the different ranges (although this is not entirely certain). Does anyone know what values are equivalent between the two? TurfIt wrote earlier about using both with default values, but I am not sure what those are (unless TurfIt meant the Simutrans default value as in the supplied simuconf.tab?)
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

Quote from: prissi on December 01, 2020, 07:04:45 AM
But zlib at level 3 is way way faster than zstd with files about 10% larger. And bzip2 was not much slower than zstd in my tests. Apparently it strongly depends on the actual zlib lib one was using ... (I had to use own compiled zlib for MSVC, maybe that is the reason.)
MSVC vs MinGW is perhaps the problem.  Question is do you have an abnormally fast zlib, or slow zstd?
For versions, mingw-w64-x86_64-bzip2 1.0.8-1, mingw-w64-x86_64-zlib 1.2.11-7, mingw-w64-x86_64-zstd 1.4.5-1.

I see the dom28.sve I have came from your collection, so I repeated tests with it. Times are the average of 5 saves. What results do you get?
   bzip2,        14382 ms to save, 29.2 MiB savefile size.
   zlib level 6,  8464 ms,             49.8 MiB
   zlib level 3,  3376 ms,             55.8 MiB
   zstd level 6, 2235 ms,             47.9 MiB
   zstd level 3, 1507 ms,             49.7 MiB


Quote from: jamespetts on December 01, 2020, 12:49:53 PM
Yes, this is an inference from the different ranges (although this is not entirely certain). Does anyone know what values are equivalent between the two? TurfIt wrote earlier about using both with default values, but I am not sure what those are (unless TurfIt meant the Simutrans default value as in the supplied simuconf.tab?)
Defaults for the libraries are both '6', same as Simutrans default. There is no direct equivalent, depends on the data being compressed. Sometimes level 6 for both give identical sizes, sometimes one or the other is bigger for a given savefile. Generally 6 is balanced, above trades time for size, and below better speed for bigger.


Quote from: jamespetts on December 01, 2020, 11:36:39 AM
Ranran and TurfIt - thank you for testing: that is helpful. TurfIt - what linking problems are you having with Extended?

It remains unclear why TurfIt's tests from last year seem inconsistent with the results that we are seeing now in Extended.
What times do you get with Standard and dom28.sve?  (Presumably prissi can point you where it is if you don't have.)
Would be interesting if you could compare a self built MSVC with the release on sourceforge too? (or any GCC build). I roughly repeat my results above when using the sourceforge vs self built  (stop watch times rather than ingame, subjectively release is 20% slower than self built - debug build maybe...)
I used this for timing:

Index: simworld.cc
===================================================================
--- simworld.cc (revision 9461)
+++ simworld.cc (working copy)
@@ -4696,6 +4696,8 @@
void karte_t::save(const char *filename, bool autosave, const char *version_str, bool silent )
{
DBG_MESSAGE("karte_t::save()", "saving game to '%s'", filename);
+uint32 zzz=dr_time();
+
        loadsave_t  file;
        std::string savename = filename;
        savename[savename.length()-1] = '_';
@@ -4727,6 +4729,7 @@
                reset_interaction();
        }
        display_show_load_pointer( false );
+printf("Save took %u\n", dr_time()-zzz);
}




As for MinGW building SimEX:

R:/build/bauer/brueckenbauer.o:brueckenbauer.cc:(.text+0x55ee): relocation truncated to fit: R_X86_64_PC32 against undefined symbol `TLS init function for karte_t::marker_index'
R:/build/bauer/brueckenbauer.o:brueckenbauer.cc:(.text+0x560b): relocation truncated to fit: R_X86_64_PC32 against undefined symbol `TLS init function for karte_t::marker_index'
R:/build/bauer/tunnelbauer.o:tunnelbauer.cc:(.text+0x3d56): relocation truncated to fit: R_X86_64_PC32 against undefined symbol `TLS init function for karte_t::marker_index'
R:/build/bauer/tunnelbauer.o:tunnelbauer.cc:(.text+0x3d78): relocation truncated to fit: R_X86_64_PC32 against undefined symbol `TLS init function for karte_t::marker_index'
R:/build/bauer/wegbauer.o:wegbauer.cc:(.text+0x7213): relocation truncated to fit: R_X86_64_PC32 against undefined symbol `TLS init function for route_t::MAX_STEP'
R:/build/bauer/wegbauer.o:wegbauer.cc:(.text+0x7265): relocation truncated to fit: R_X86_64_PC32 against undefined symbol `TLS init function for karte_t::marker_index'
R:/build/bauer/wegbauer.o:wegbauer.cc:(.text+0x726f): relocation truncated to fit: R_X86_64_PC32 against undefined symbol `TLS init function for karte_t::marker_index'
R:/build/bauer/wegbauer.o:wegbauer.cc:(.text+0x72e4): relocation truncated to fit: R_X86_64_PC32 against undefined symbol `TLS init function for route_t::max_used_steps'
R:/build/bauer/wegbauer.o:wegbauer.cc:(.text+0x73bd): relocation truncated to fit: R_X86_64_PC32 against undefined symbol `TLS init function for route_t::max_used_steps'
R:/build/bauer/wegbauer.o:wegbauer.cc:(.text+0x73d7): relocation truncated to fit: R_X86_64_PC32 against undefined symbol `TLS init function for route_t::max_used_steps'
R:/build/bauer/wegbauer.o:wegbauer.cc:(.text+0x7615): additional relocation overflows omitted from the output
collect2.exe: error: ld returned 1 exit status
make: *** [common.mk:27: /r/build/Simutrans-Extended.exe] Error 1

A brief look into possible causes was enough to no no no nope. Not going there.
It worked in January when I did the original zstd testing, so something broke. Until it either self fixes, or someone else fixes, no SimEX for me.

jamespetts

TurfIt - I do not use MinGW, but rather either MSVC for my home debugging builds or GCC for the automated server builds. I am not sure why you are getting those odd linker errors (which suggest some 32/64 bit incompatibility somewhere; are you trying to build a 32- or 64-bit version? If the former, I suggest trying a 64-bit build, as this is necessary for the level of memory consumption of larger games in any event).

I have not tried loading/saving the Standard saves, mainly because the case that I specifically want to test is loading/saving a large amount of path explorer data, which is what Zlib has trouble with in particular. Saved games not containing these data are thus of only limited value for testing.

I should note that I use MSVS's performance profiler for timing purposes, as the CPU utilisation graph makes it very obvious when loading/saving begins and ends (a very long, single-threaded section of steady CPU utilisation characterises loading/saving, compared to a spiky, semi-multithreaded section for the running game).

The information as to the defaults is useful - thank you. It does seems as though it is worth exploring reducing the compression levels to see whether this improves performance.
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

I'm aware standard does not have path explorer data to save, and this data is what appears to choke zlib saving, at least when tested in January.

I suggested testing with Standard since prissi is reporting zstd slow in Standard, same as you reporting zstd slow in Extended. Yet I see very tangible improvements in both (albeit back in January for Extended since I currently can't compile it). Phystam also reported a good improvement with Extended back in January.

If you test Standard from sourceforge and get slow results, then I don't know. If you roughly match my results, and then you try with a MSVC built Standard showing slow zstd, then there's the problem. I wouldn't call that 'limited value for testing'.  Or, since you are using GCC for automated server builds, test with them.  (Are your nightlies from GCC? If so, can you make a nightly run with the zstd branch? [then I could try that...])

TurfIt

#231
Results with Extended, merge-from-standard-zstd branch, bb-17-jan-2020.sve:
  bzip2, 382453 ms,   958 MiB
  zlib 6, 227670 ms, 1162 MiB
  zlib 3,   71905 ms, 1308 MiB
  zstd 6,  41733 ms, 1164 MiB
  zstd 3,  24686 ms, 1150 MiB  (yes smaller than 6)

1. This branch will not load the current Bridgewater savefile - presume a versioning issue. Stuck in first call to stadt_t::rdwr(loadsave_t* file).
2. SDL2 crashes after pakset selection. GDI working. (I remember this problem from January too...)
3. Branch makefile for ZSTD doesn't work.  Typo ZSRD, and if USE_PNP where it don't belong. Results in zstd not being included - defaults to bzip2 saving when set saveformat=zstd. Fix makefile, zstd works.
4. Relocation truncated "solved" with linker flag fuckery: "LDFLAGS = -Wl,--image-base -Wl,0x10000000". Apparantely you've managed to get the code/static allocations over 2GiB.
5. bb-17-jan-2020.sve is sucking down 14.5 GB memory usage when loaded. Once OS overhead involved, even players with 16GB system ram will be in bad shape.

EDIT:
6. Loading takes 27 seconds. All compression formats the same.
7. Save progress bar should probably take saving the path explorer data into account - once regular data is saved, progress bar is at the end where it appears the saving process is frozen while it saves the PE data.

EDITEDIT:
Results with gzbuffer 64K, and LS_BUF_SIZE 8M
  zlib 3,   65211 ms, 1308 MiB
  zstd 3,  23856 ms, 1150 MiB

8.  zlib definitely likes more than 8K buffer space.  zstd in diminishing returns already, could try different sizes, but note this will be system dependent for optimal.

Ranran

Quote from: TurfIt on December 01, 2020, 04:52:51 PMTypo ZSRD
It was due to a standard commit. It was brought in r8888 and fixed in r8905, but I missed it because it looked like a completely irrelevant commit...

jamespetts

Thank you very much for this. I was going to have a go at compiling this branch on the server to test how well that zlib cross-compiling works, but you have managed to get it working before I had time to do that.

These results are interesting (and odd in so far as compression level 3 is better than 6 for zstd, but it at least shows that there is no advantage to increasing the ratios). Testing with a GCC cross-compile build is in order, I think, when I have the time to set this up.

As to 1, the incompatibility is a known issue, discussed above, which should be resolved before deployment. In relation to 2, I have not experienced this myself nor had any other reports of this, so I am not sure how to reproduce this to look into it.

3 - thank you for pointing out the error; this will need to be fixed.

4 - interesting. I am not sure why this is so large.

5 - yes, that is why we moved to a smaller number of towns and slower town growth for the current game.

6 - I am not sure how this is consistent with the findings above - perhaps I am misunderstanding?

7 - I can see that this would be preferable, resources permitting.
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

See my EDITEDIT  re buffers.

6. You'd stated "loading using the existing Zlib took 33 seconds, whereas loading with the zstd took 1:23 minutes."  For me, I load the bb-17-jan-2020.sve savegame in 27 seconds regardless of whether the file is in bzip2 format, zlib format, or zstd format.


Quote from: Ranran on December 01, 2020, 05:33:04 PM
It was due to a standard commit. It was brought in r8888 and fixed in r8905, but I missed it because it looked like a completely irrelevant commit...
Actually not fixed until 8910. And I agree annoying these stowaway fixes.

jamespetts

Quote from: TurfIt on December 01, 2020, 05:47:51 PM
See my EDITEDIT  re buffers.

My apologies: that crossed with my reply, I think.

Quote
EDITEDIT:
Results with gzbuffer 64K, and LS_BUF_SIZE 8M
  zlib 3,   65211 ms, 1308 MiB
  zstd 3,  23856 ms, 1150 MiB

8.  zlib definitely likes more than 8K buffer space.  zstd in diminishing returns already, could try different sizes, but note this will be system dependent for optimal.

That is very interesting. There is not a currently implemented simuconf.tab option to modify the buffer size; is this a parameter that can be passed to a function somewhere? If so, it would help to know what it is so that I can look into adjusting this.

Thank you again.
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

No simuconf.tab for these, hardcoded.
LS_BUF_SIZE is a constant at top of loadsave.cc
gzbuffer is added to loadsave_t::wr_open: 

if(  is_zipped()  ) {
// using zlib on servers, since 3x times faster saving than bz2
char compr[ 4 ] = { 'w', 'b', (char)('0' + clamp( level, 1, 9 )), 0 };
fd->gzfp = dr_gzopen(filename_utf8, compr );
gzbuffer(fd->gzfp, 65536);
}

jamespetts

Thank you for that. Which of the parameters comprises the buffer size - is it the compr parameter passed to gzfp, which object is in turn passed to gzbuffer?
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

65536 is buffer size. 8192 is default if no call made to gzbuffer() after gzopen().


/ buffer size for read/write - bzip2 gains up to 8M for non-threaded, 1M for threaded. binary, zipped ok with 256K or smaller.
// zstd need their own buffer size ...
//#define LS_BUF_SIZE (1024 * 1024)
#define LS_BUF_SIZE (8192 * 1024)

and 1M or 8M here for the simutrans buffers (which zstd has hijacked too...)

jamespetts

Excellent, that is very helpful, thank you. I shall have to experiment when I have time.
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

So it seems like zstd is something of a sweet spot? Because zlib in all my trials were like 10-20% bigger but twice as fast. Of course on my computer, so things may vary a bit. (I am not sure how well zstd is supporting threads. The slow test computer had only a deliberately slow dual core ancient CPU. Also instruction set may greatly speed up compression.)

Ranran

https://forum.simutrans.com/index.php/topic,20492.html
The above thread function has been incorporated. It is useful for pakset authors to update the menuconf to make this feature available.

jamespetts

Thank you very much for that.I have now managed to build the zstd version on the Bridgewater-Brunel version, including cross-compiling a 64-bit Windows build, which can be downloaded for testing here.

However, there is still an incompatiblity with saved game versions from the latest master branch: see here for an example, which is the game that I have been using for testing.

Brief tests show that zipped with compression level=3 is much, much, much faster than zstd with compression level=3 (tens of seconds as opposed to several minutes; I have not at this stage done any actual timing, as it is more difficult to do this when not running in the debugger), and even with zipped level 6, this appears faster than zstd level 3, although this is not as obvious.

In any event, given the improvements that being able to specify the level of compression has, it is probably worthwhile moving towards integrating the latest set of changes as soon as possible. Further testing as to the optimum setting can be done after integration. The main things that will need to be done first, so far as I can make out, are (1) fixing the saved game incompatibility issue; and (2) dealing with the translation texts.

Ranran - can I check that all the necessary texts that I need to translate for the new UI work are listed in this thread?

Also, can I check whether you think that there is anything else that needs to be done before integrating the UI improvements together with the zstd work into the master branch?

Thank you again for all your work on this - it is much appreciated.
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

#243
Quote from: jamespetts on December 02, 2020, 07:14:43 PM
Thank you very much for that.I have now managed to build the zstd version on the Bridgewater-Brunel version, including cross-compiling a 64-bit Windows build, which can be downloaded for testing here.

ERROR: loadsave_t::rd_open: Compiled without zstd support!

Oops. Note, in this case, if your testing was with this exe, when you select saveformat=zstd, you actually get bzip2 which is very very slow...



Using my exe, and bb-nov-2020.sve I get:  (on an I7-6700K - previous timings above were on a 2700X)
   zlib 6, 81509 ms, 424 MiB
   zlib 3, 30017 ms, 506 MiB
  zstd 6, 18105 ms, 401 MiB
  zstd 3, 13553 ms, 401 MiB

And with bb-17-jan-2020.sve:  ( for a point of comparison 6700K vs 2700X)
   zlib 3, 74311 ms, 1310 MiB
  zstd 3, 23726 ms, 1150 MiB

Ranran

Quote from: jamespetts on December 02, 2020, 07:14:43 PMHowever, there is still an incompatiblity with saved game versions from the latest master branch: see here for an example, which is the game that I have been using for testing.
Only the conflicts in the 8653-2 branch were fixed and the conflict in the zstd branch was not fixed. I think I fixed it.

Quote from: jamespetts on December 02, 2020, 07:14:43 PMRanran - can I check that all the necessary texts that I need to translate for the new UI work are listed in this thread?
Some translated text has been added by a recent change in the sound setting dialog built into the zstd branch. I updated the list.
Some help files also need to be added or reviewed.