PaperMC / Paper

The most widely used, high performance Minecraft server that aims to fix gameplay and mechanics inconsistencies
https://papermc.io/
Other
10.02k stars 2.32k forks source link

joml enable approximations #10092

Closed NeumimTo closed 10 months ago

NeumimTo commented 10 months ago

Is your feature request related to a problem?

from javadocs

Contains fast approximations of some Math operations. By default, Math methods will be used by all other JOML classes. In order to use the approximations in this class, start the JVM with the parameter -Djoml.fastmath. There are two algorithms for approximating sin/cos: arithmetic polynomial approximation contributed by roquendm theagentd's linear interpolation variant of Riven's algorithm from http://www.java-gaming.org/ By default, the first algorithm is being used. In order to use the second one, start the JVM with -Djoml.sinLookup. The lookup table bit length of the second algorithm can also be adjusted for improved accuracy via -Djoml.sinLookup.bits=, where is the number of bits of the lookup table.

Describe the solution you'd like.

set system property -Djoml.fastmath and -Djoml.sinLookup.bits during paper startup

Describe alternatives you've considered.

Provide another math library with paper thats better than java math, for example on sponge platform we have access to https://github.com/SpongePowered/math without need to do any kind of shading.

Other

No response

lynxplay commented 10 months ago

I think that choice should probably be left to the server administrators ? For plugins that use a lot of trig functions, them shading in a minimal library to create these lookup tables for them seems easy enough ?

Would love the opinion of other people tho 👍 I have not looked through joml's fastmath approximations, I presume sin/cos/atan might have changes, the approximation range of those and potential limitations of input values.

Given joml is exposed to the API, messing around there is a tad dangerous if contracts break.

olijeffers0n commented 10 months ago

This seems like something that the server owner can easily enable, i’m not sure why would Paper enable it by default?

NeumimTo commented 10 months ago

I see several problems with this approach

olijeffers0n commented 10 months ago

I think this may be better suited to a docs page, with some actual graphs of how this will affect performance and whatnot, rather than forcing this. If it is possible to enable it by programmatically setting the property, this could be a configuration option.

lynxplay commented 10 months ago

Yea I don't think getting server owners to flag this is the best approach. IMO the plugin should be responsible for providing their own lookup table.

Again, to enable this, we'd need to ensure that none of the fastmath approximations fail to cover all cases the normal methods would as JOML is exposed to plugins via API.

From a quick glance their approximations are just that, approximations. Good ones sure, fast ones as well. Their atan2 implementation e.g. does not handle 0/0 as an argument and yields NaN instead of 0.

Changing behaviour like this for plugins that might be using this already seems problematic just so that plugins can use their provided sin lookup tables when those are easily self-provided via third party libs shaded or copy-paste.

Owen1212055 commented 10 months ago

See comments listed above. In general, there is no probable reason why Paper should be manually enabling this, and because this again causes issues in itself due to now causing these joml operations to now be approximations, which is breaking behavior.. this will be closed.

Thank you 😄