FastAsyncWorldEdit icon indicating copy to clipboard operation
FastAsyncWorldEdit copied to clipboard

feat: improve fawe limits (#2773)

Open dordsor21 opened this issue 1 year ago • 1 comments

  • add FaweLimit implementations for increasing concurrency levels
  • allow FaweLimit to perform processing (and forcefully disable as required) to capture [tile] entities
  • Use BlockVector3#set(Extent orDefault) where appropriate to reduce block checks
  • fixes #2679
  • fixes #1874
  • take 2 looks like it should work fine

dordsor21 avatar Jul 28 '24 18:07 dordsor21

See dea1fcf for fix

dordsor21 avatar Jul 28 '24 18:07 dordsor21

So, changing from Extent#method(BV3) to BV3#method(Extent) might skip the LimitExtent, but only when processing? If so, that's fine, and should bring a nice performance improvement in general for everything that reads from the world (some patterns, most masks).

I'm not sure about the different limit implementations. Are they needed? How relevant would be the overhead when just having a limit using Atomic* types? We could also probably use VarHandles to have more control over memory ordering while also reducing objects needed in that case. Does the ProcessorFaweLimit actually improve performance? The lambda in updateAndGet might be called multiple times, while also writing to a volatile variable...

Similarly, the SynchronousSettingExpected annotation and the boolean flags feel a bit fragile and more like band-aid. Those are also only needed because of the different limits, right?

SirYwell avatar Sep 11 '24 15:09 SirYwell

In fairness where we're likely to see the speed matter is concurrent-capable operations that are already fast, i.e. set and replace, where we'd be using the concurrent limit anyway so it's probably worth just collapsing to atomic.

I don't think VarHandles can offer us much more than we're getting with Atomic. We need to be sure of both the read and write at the same time and the read only matters "once" as the edit is consequently exited so I would say the simplicity of Atomic is worth it. The memory overhead is also next to nothing

dordsor21 avatar Sep 14 '24 20:09 dordsor21