Drasticly improve zoom tile calculation for larger maps when using MySQL/MariaDB storage Engine

[stormboomer]

…SQL storage engine. For Larger Tables doing Limit and Offset can have a big Impact on Statement execution because it is an IO heavy task.

This fixes the issue by not doing any limit / offset on the SQL statement but instead query for all the data at once, and let the JDBC handler do the resultset handling. This can probably be adapted for MSSQL, PostgreSQL and SQLLite.

[stormboomer]

I did some further digging and found out that we are able to improve overall render performance drasticly for multicore rendering.
It seems that the ImageIO devs fixed some really old problems with ThreadSafety https://bugs.openjdk.org/browse/JDK-6986863

in ImageIOManager we can remove all

synchronized(imageioLock) {
...
}

Statements.

So far simple tests work on my machine, but I am unsure to make the change within this PR, let me know how to procede further.

[mikeprimm]

Fix https://bugs.openjdk.org/browse/JDK-6986863 is only indicated as fixed as of JDK 17 - and only then as of 17.0.8 (build 8), which is as of July, 2023. This code is common across a bunch of versions still only supported on JDK8 and JDK16, and no doubt a bunch of even current server are not running a July 2023 patched JDK, besides needing to see if this fix is also in Oracle JDK.
We could do this, but only with corresponding version checks - not telling existing users that they need to convince their hoster to patch JDK just for this...

[stormboomer]

Yes, I gues you are right on that one.
I will have a look at it tomorrow and provide a suggestion within this PR.

[mikeprimm]

Yeah - I'm gonna look into it separately: the ImageIO thing is a 'day zero' problem in the JDK, and while it's great to see that someone actually got around to fixing it, I just want to be sure to not do something that will break servers and/or result in broken tile renders without really confirming that the JDK being run on is REALLY fixed

But, thanks for finding it! :)

[stormboomer]

I think this should improve the overall performance.
TLDR:

1. it seems that we do not have to check for a specific JDK 17 version, the oldest version I am able to get works fine.
2. Works with both OpenJDK and Oracle JDK
3. Overall Rendertime improves by 8-10% for Filestorage and MySQL (both testet)
4. Zoom Processing improves by 18-19%

For test Results see
DynmapTestResults.zip

This is a test for improved render speed overall AND better render speed for MySQL/MariDB storage engine.
Example Config can be found in attachment.
Special Build was used. Included more logging for Enum and Zoom rendering, to get a better feeling when everything is completed
Logfiles of each run can be found as screenlog - contains ALL outputs and inputs I did during the run.

Test was done on purpose with 2 old JDK17 versions, to avoid conflict if users are not using up to date versions

• Oracle JDK 17 https://www.oracle.com/java/technologies/javase/jdk17-archive-downloads.html
• Open JDK 17 https://jdk.java.net/archive/
• Dynmap was run with MySQL/MariaDB storage engine
• a random generated world (no addiational chunks just default) was used
• example Config is provided
• a final comparison test to compare filestorage and MySQL was done on Oracle JDK 17 by switching the configuration back to filestorage

Test was done on a LXC Container

• 10GB Memory Total
• 16 Cores, Host is running a Ryzen 9 5950X
• server is started with 6G memory and Debugger attachable -Dcom.sun.management.jmxremote -Dcom.sun.management.jmxremote.port=$DebuggerPort -Dcom.sun.management.jmxremote.authenticate=false -Dcom.sun.management.jmxremote.ssl=false -Djava.net.preferIPv6Addresses=true -Xms6G -Xmx6G -XX:+ParallelRefProcEnabled -XX:MaxGCPauseMillis=200 -XX:+UnlockExperimentalVMOptions -XX:+DisableExplicitGC -XX:+AlwaysPreTouch -XX:G1NewSizePercent=30 -XX:G1MaxNewSizePercent=40 -XX:G1HeapRegionSize=8M -XX:G1ReservePercent=20 -XX:G1HeapWastePercent=5 -XX:G1MixedGCCountTarget=4 -XX:InitiatingHeapOccupancyPercent=15 -XX:G1MixedGCLiveThresholdPercent=90 -XX:G1RSetUpdatingPauseTimePercent=5 -XX:SurvivorRatio=32 -XX:+PerfDisableSharedMem -XX:MaxTenuringThreshold=1 -Dusing.aikars.flags=https://mcflags.emc.gs -Daikars.new.flags=true

Explanation of the Changes:

1. in the class MySQLMapStorage the Method processEnumMapTiles can cause high Database Load by doing Selects with Limit and Offset Clauses.
   This was most likely done to provide some sort of "chunked" Processing, and don't stream large Datasets from the DB Server.
   Sadly once the Tiles table grows larger (can happen fast) these Limit and Offset clauses can cause the DB Server to produce absurdly high IO load on the system.
   If you are running the DB server on the same Host as the MC server, it can even impact the MC server.
   Lukily we can easily fix this by letting the JDBC Connector handle the Dataset in Chunks. Therefor asking the DB only once and then stream the Data in chunks.
   This also improves Processing time on smaller renders, as there are fewer network / DB calls done.

2. The ImageIOManager is implemented with sepcific Thread Safety mechanisms that cause a multi threaded system to spend much time in waiting for a lock.
   Luckily in JDK 17 it seems that this is not needed anymore.
   It seems that it was fixed with https://bugs.openjdk.org/browse/JDK-6986863 Sadly it is not clear on which version the fix was rolled out -> therefore we are testing it.
   The oldest JDK 17 I am able to download is build (17+35-LTS-2724) from my understanding that would be build 35 and would therefore contain the mentioned improvement. My tests for this version are not showing any problems.

Compare Oracle JDK 17 improvements
The change improves the total render time (start: loading texture pack - end: last zoom out done) by 57s which is a 9.08% improvement.
We have an average improvement of 1.27ms per rendered tile and a 57.97ms improvement per Render.
In percentages that is a a 13.46% improvement in per tiles rendering and a 17.90% improvement per Render
Noticable is that for Flat maps (iso_S_90_hires) the improvement is much bigger (probably because the tiles can be rendered faster) with an average of 36.44%.
For Surface maps (iso_SE_30_hires) the improvement is not that big with 7.47%
Taking a deeper look at what the original improvement goal was - the processEnumMapTiles - we get an improvement by 72.22% which is a huge improvement
Zoom out processing is improved by 18.29%

[mikeprimm]

In past VMs, ImageIO.write was just as thread unsafe - the changes, as is, remove the thread safety on writing unconditionally while adding the safety check back in only on ImageIO.read. Also, testing on 17 is all good, but the real tests are on JDK8 versions (which is basically anything before MC 1.17), since those are the versions that the change would break if done incorrectly.

[stormboomer]

Are we still compiling for JDK8? When I download the latest version from https://legacy.curseforge.com/minecraft/bukkit-plugins/dynmap/files/4632182 it will not work with JDK8 and paper-1.16.5-794 telling me that it was compiled by a later version.

[18:40:55 ERROR]: [org.bukkit.craftbukkit.v1_16_R3.CraftServer] org/dynmap/bukkit/helper/v120/BukkitVersionHelperSpigot120 has been compiled by a more recent version of the Java Runtime (class file version 61.0), this version of the Java Runtime only recognizes class file versions up to 60.0 initializing dynmap v3.6-899 (Is it up to date?)

So the latest version can not be used anyway by pre 17 JDK. btw I am also unable to start using JDK 16

Additional Info: The last version that I am able to run with JDK8 is Dynmap v3.5-873 from https://legacy.curseforge.com/minecraft/bukkit-plugins/dynmap/files/4512871

[stormboomer]

Reason for the problem is that this line

sourceCompatibility = targetCompatibility = compileJava.sourceCompatibility = compileJava.targetCompatibility = '1.8' // Need this here so eclipse task generates correctly.

is missing in DynmapCoreAPI/build.gradle

I build a local version that works with JDK8 and paper-1.16.5-794. I verified that the version check I have implemented works - therefore on older JDK Versions we still use synchronisation.

Do you want me to include the change to the DynmapCoreAPI/build.gradle with this PR?

[mikeprimm]

Yep - everything in the core is jdk8 (language level 8, more accurately) - platform specific components are built with language level appropriate to them. It's all built using JDK17, but the language level controls the op code generation. See https://github.com/webbukkit/dynmap/blob/v3.0/DynmapCore/build.gradle#L11 and the like for how this is done in each gradle component

[mikeprimm]

Yep - the level missing in the API module is incorrect, and should be 8.

[stormboomer]

Ok, have added this change to the PR as well.

[mikeprimm]

Thanks - had to recently make some other changes to accomodate those paper builds (didn't see problem on spigot...): suspect particular to some custom loader stuff - but oddly enough didn't see that problem when I tested current builds with 1.16.5 and openjdk8.

[mikeprimm]

Cool - I'll give PR a proper review tonight, and get it pulled - thanks for the effort!

[mikeprimm]

Thanks!