fix(key): Use String#intern in KeyImpl

[kermandev]

I changed the KeyImpl to use a String#intern to save on memory usage with the same key objects created, a better option than using a key pool, and should be faster to compute #equals by reference checks for long-lived keys.

Motovation

Minestom recently migrated to using keys from its namespace type; From this change, we saw an increased memory usage of about ~4MB. This was due to our old implementation using an intern for the namespace and value. This increase isn't great. This is testable on the demo server available in Minestom's repo, and we hold about 39k instances of Key at runtime from its registries. After this change, we went from 5.42MB to 1.1MB retained from KeyImpl.

This issue could be primarily mitigated in Minestom internals by dropping the default namespace. Still, any user could do this with any namespace, and since we bundle adventure to our users, this is not an applicable option.

Another solution

The adventure could change the parser to use intern instead after substrings, but users could still inadvertently create many long-lived equal strings.

Let me know your feedback on this change and if it breaks any style guides that Adventure may have.

[kezz]

Hi! Thanks for the PR! We've had a discussion internally and would like to instead implement this via a system property, that sets whether NONE, NAMESPACE or ALL parts of the key are interned. You can see our AdventureProperties internal helper class for creating system properties (probably would want it to be key.internKeyParts) for a pointer to this.

[kashike]

adventure.internKeyParts

[kermandev]

Wouldn't creating a shared properties package between the two at that point be a better solution than two separate properties API's (as the key doesn't require adventures api package)?

I would think pulling out the properties package from the internals would be nicer than implementing the same system twice, and the property would be under net.kyori.adventure.internKeyParts using that system

[kermandev]

I've provided an example of the cryptic wording I used above; please let me know your feedback and the correct default value that has been decided. (I'm assuming NONE is good enough to provide backward compatibility).

Just to let you know, there is still a TODO on rewriting tests to follow the strategy used; I am unsure if I should expose the internal intern strategy. (Maybe move this property entirely under Key/KeyImpl vs AdventureProperties) and expose the enum there, but not all implementations of Key will ever have to have this strategy. It's only for the current implementation.

[kermandev]

This should be good to review; I don't know what way you would like the test structured as all of the properties in Adventure are also stored as final in their implementations.

[kezz]

Just a few minor changes!

[kezz]

Thanks!