Bukkit/Bukkit — pull request #812 by crast (2013-03-21 06:21:33+00:00)
Title: [B] Don't cache metadata store disambiguations [BUKKIT-3841]

This is in reference to the cachedDisambiguate method visible here: https://github.com/Bukkit/Bukkit/blob/a2566bfc3e1bb845fd2e8566dee738630c3886b1/src/main/java/org/bukkit/metadata/MetadataStoreBase.java#L121-L142

The method makes the assumption that caching the value will provide a performance improvement. In following the principle that premature optimization is well, simply that, I have written a series of tests showing that the break-away point upon which this caching is beneficial is only on a very high hit rate (~90%). Due to the common use cases of Metadata, I don't forsee this being all that useful, especially given most of the values that might be cached have a very short lifecycle in actuality (I can explain this in full if required, but suffice it to say it's not caching nearly as much as it seems it is). In addition, being a multi-level hashmap this uses a considerable amount of memory.

The performance tests I did in another branch: https://github.com/crast/Bukkit-Work/commits/metadata-cachedisambiguate-perf The tests can be run for yourself by checking out this branch, running mvn package; and then bash run_tests.cmd (linux/OSX) or run_tests.cmd (windows)

The output of the tests (run on OSX, x86_64, Java SE 6 1.6.0_37):
```
About to run speed tests.
Running... (10000/30)
#########
Presumed hit rate: 0.0%
Uncached: 1 loops
-> elapsed: 172ms, N: 33330
Cached: 1 loops
-> elapsed: 539ms, N: 33330
#########
Presumed hit rate: 50.0%
Uncached: 2 loops
-> elapsed: 298ms, N: 66660
Cached: 2 loops
-> elapsed: 729ms, N: 66660
#########
Presumed hit rate: 75.0%
Uncached: 4 loops
-> elapsed: 429ms, N: 133320
Cached: 4 loops
-> elapsed: 626ms, N: 133320
#########
Presumed hit rate: 87.5%
Uncached: 8 loops
-> elapsed: 1036ms, N: 266640
Cached: 8 loops
-> elapsed: 1144ms, N: 266640
#########
Presumed hit rate: 93.75%
Uncached: 16 loops
-> elapsed: 1640ms, N: 533280
Cached: 16 loops
-> elapsed: 1289ms, N: 533280
#########
Presumed hit rate: 96.875%
Uncached: 32 loops
-> elapsed: 2620ms, N: 1066560
Cached: 32 loops
-> elapsed: 2279ms, N: 1066560
#########
Presumed hit rate: 98.4375%
Uncached: 64 loops
-> elapsed: 5165ms, N: 2133120
Cached: 64 loops
-> elapsed: 4476ms, N: 2133120
#########
Presumed hit rate: 99.21875%
Uncached: 128 loops
-> elapsed: 10822ms, N: 4266240
Cached: 128 loops
-> elapsed: 8968ms, N: 4266240
===== DONE
```

As you can see, that with a very high hit rate, we actually do get a speed improvement over un-cached. In this scenario it's keeping 10,001 hashmaps (one hashmap with 10,000 keys, each entry of which is a hashmap with 30 keys) and thus the memory usage is quite notable. I am working on an isolated memory test which will highlight the memory usage of the datastructure more concretely, but suffice it to say it's a pretty substantial memory usage, making this a very poor time/space tradeoff.

I will also post tests on Linux on java SE 7 soon for comparison.