#89: Removed relocation checking

[Andret2344]

@Bastian please, take a look

Implemented #89

[Bastian]

Hey,
do you know where I can find some more information about the inner workings of this feature?
The linked Wiki article is very short and does not contain a lot of information.

The underlined text sounds like it shouldn't be used for something like bStats.
And also in the initial announcements (Link), there isn't a lot of information.
My biggest problem with it is the unexplained "in some circumstances":

Under which circumstances? It's not unlikely for a server to have a few dozen plugins that use bStats, all in different versions. Conflicting bStats versions would be a huge issue in these cases that could potentially break many plugins.

I cannot recommend (*) to use the library loader without being 100% sure that it wouldn't break anything.

(*) In fact, I don't even allow it! YOU MUST NOT USE A BSTATS VERSION WITH THE REMOVED RELOCATE CHECK!

[Andret2344]

Ok, let me explain why we (as developers) don't need relocation checking.

Since 1.16 spigot implemented availability to list libraries that our plugin uses to download them from Maven Central. The main problem that was occurring is that if some library (like, i.e. org.json:json) was used by, let's say, 20 plugins, each of those plugins was required to have org.json:json included and relocated, because otherwise there would appear exceptions with duplicated classes. Additionally, each plugin was bigger than its real code due to the requirement to contain all the dependencies it uses. If 20 plugins uses 10 exactly the same dependencies, each of those 10 dependencies was loaded to server memory 20 times. It makes the server inefficient and plugins to be fat.

Now, we can list the dependencies we need with its exact versions. Server downloads it and provides, so plugins don't need to make plugins fat and relocating anything, because server downloads the dependency once and takes care of everything of the dependency. If our 20 plugins uses 10 libraries (for simplicity, let's say that all dependencies' versions matches), every dependency is downloaded and loaded to server memory exactly once.

If the developer creates the "fat jar" with dependencies, we can be sure, that he knows and understands the relocation mechanism and he/she shouldn't be forced to use it. Now, bStats forces to relocate plugins, what means if 20 plugins on my server uses bStats:3.0.0, server must load the same code 20 times (relocated each time) instead of load it once and provide to all plugins. What's the point?

If it comes to the "circumstances", it means that if one plugin requires org.json:json:20201115 and another requires org.json:json:20220320, the server is able to download both versions and provide correctly due to the libraries YAML declaration.

Huge servers have more than 100 plugins, and in the pessimistic scenario, each of those uses bStats, so the server must load all the used classes more than 100 times instead of once (or a few, depending on versions used).

Now it's even possible for a plugin to use bStats:3.0.0 and the other to use bStats:2.2.0, and there's totally no difference between providing them in the fat jar and declaring them in the YAML. Disallowing to use the second option makes the server to consume a bit more memory, and requires developers to usually take special care about only this one dependency.

Ok, I understand that we aren't 100% if it wouldn't break anything, so... maybe we should test it? On 2, 3, 5, 10 different plugins, using different versions to see how does it behave and if it is still working as expected, or not.

Please, keep in mind that most of the developers get furious when they try to use the dedicated solution for loading dependencies instead of relocating them and get exception that one dependency is not relocated. Especially when developers use the new feature to quit relocation, forcing them to stick to this is simply poor.

Don't understand me wrong, I'm not saying that you are somehow obliged to allow skipping relocation, but it's not something that should be ignored as an idea. As I wrote above, it should be widely tested and verified if it can be used via YAML declaration, or it really breaks it. Sh*t happens, and we can find out that YAML configuration does break the mechanics, and that's ok, but it would be really more than welcome to mention it on README.md file, that we should not even try to do so.

Don't be mad on us, we just want to include bStats as every other library from available in Maven Central and don't fully understand reason behind your denial of this initiative.

[mdcfe]

The benefit of using the Spigot library downloader is that the server can download and load large dependencies once. bStats is a tiny dependency when shaded, so this benefit is entirely moot here - the time it takes to download the library (no matter how short it is) will outweigh however many bytes you shave off your jar size.

In addition, Spigot's classloader behaviour is undocumented and unpredictable and varies wildly between the different MC versions supported by bStats. Endorsing the usage of unrelocated bStats classes in any form (both shaded and runtime downloaded) is likely to cause havoc, both on versions as old as 1.8.8 and as recent as 1.18.1.

Ultimately, nobody is forcing you to go all-or-nothing with Spigot's library downloader. You can use it for larger dependencies while still shading the miniscule classes like bStats into your jar.

[kennytv]

Most of what you said here doesn't make sense. Especially new developers should be taught the necessity of relocating dependencies. Usually, the larger a server is, the less plugins it has, mostly being self-made and specifically catored plugins. The comparison to something like a json library also doesn't make sense considering metrics-lite is nothing but a single class and has absolutely zero impact on memory or performance on your server, even if used in 100 different plugins.

While this would likely work on Paper servers, Spigot's plugin classloader is awful and is unlikely to handle this very well (and will 100% fail on older versions)

[MartenM]

The spigot library loader was intended to be used for relatively large libs. For example, if your plugin was made using Kotlin.
I don't see a benefit for users/developers/server owners by using the library loader provided by Spigot. Bstats is just a single class with minimal impact.