search

Mojang / DataFixerUpper

A set of utilities designed for incremental building, merging and optimization of data transformations.
MIT License
1.2k stars 138 forks source link

Fix concurrency and performance issues #14

Closed aikar closed 6 years ago

aikar commented 6 years ago

When implementing asynchronous chunk loading, numerous concurrency issues were found.

These changes have been in use in Paper's Async Chunk system now and have no more known issues. We are doing chunk conversions over the thread pool now.

Also replaced quite a few bad uses of Map.containsKey

containsKey "reads" cleaner, but results in double the performance cost of all map operations as containsKey in MOST cases where null values are not used is identical to get() == null

Considering how deep data fixers go in call stacks, with tons of map lookups, this micro optimization could provide some real gains.

Additionally, many of the containsKey/get/put style operations were also a concurrency risk, resulting in multiple computation/insertions.

msftclas commented 6 years ago

CLA assistant check
All CLA requirements met.

aikar commented 6 years ago

I've tested the requested changes against Paper with our Async Chunk Loading, deleted my region files, and reconverted chunks and have not seen any issues arise.

Should be good to go.

aikar commented 6 years ago

Hold on I'm investigating what appears to be a server destroying performance regression with these changes.... I believe it's from the moving from static to instance property, as now types that would be considered equal wont share same cache.

aikar commented 6 years ago

Yep, Ok I'm reverting those last commits, that was it. Casting it is :)