1.10.2 Efficient way of handling block-breaking enchantments (OP-Prison)

Discussion in 'Spigot Plugin Development' started by Askingg, Jan 19, 2020.

  1. So, I'm working on one of my servers. I haven't released it yet however I fear that my blockbreaking enchantments are going to pose a threat to the server's performance.. (Especially considering currently my blockbreak event is around 200 lines long - including two blockbreak enchants. The code for the two block-break enchantments I have currently can be found at https://pastebin.com/WJzw4Azi . (I also intend on having another couple of blockbreaking enchantments for the server...

    Any ideas/suggestions?
     
    • Useful Useful x 1
  2. You can make it async and when it's doing something to sync it.

    /e Just saw that par of it already is async
     
  3. I originally coded it Async. There was an error with it though, If memory serves me correctly, it was something along the lines of cannot set block asynchronously.
     
    • Like Like x 1
  4. You make it async and when you have to like set a block run Bukkit.getScheduler().runTask(() -> block.setType(Matrial.AIR));
    for example
     
  5. I'll give that a shot, but surely that still counts it as being Async?
     
    • Agree Agree x 1
  6. In my opinion, it would be best if you create an async task for each enchantment
    -> in each if
    and when you need to do something like setting block or other synchronized task you create a new synchronized task for this specific synchronization critic operation and nothing else.
     
  7. I'm not sure we're on the same page. But what I tried is making the section with the CEs async. And then any time I had to remove a block I made a new runnable which wasn't async. However I still get the same error java.lang.IllegalStateException: Asynchronous block remove!

    However as I said, I'm not sure we're thinking of the same thing
     
    • Funny Funny x 1
  8. That's what I was trying to tell you to do.

    Code (Java):
    Bukkit.getScheduler().runTaskAsync(() -> {
      for (Block b : blocks){
        if(b.getType().equals(Material.Lava)){
          Bukkit.getScheduler().runTask(() -> {
             b.setType(Matrial.AIR);
          });
        }
      }
    });
    something like that ^^ and it should work fine
     
    • Optimistic Optimistic x 1
  9. That's essentially the short version of what I'm doing. Does it make a different that I'm doing my tasks as
    Code (Text):
    new BukkitRunnable(){
        public void run() {
            //Code
        }
    }.runTaskLaterAsynchronously
    rather than
    Code (Text):
    Bukkit.getScheduler().runTaskAsync
    ? (Should I change the way I do the runnables?)
     
    • Like Like x 1
  10. It's actually Bukkit.getScheduler().runTaskAsynchronously
    And I dont think that it really makes difference
     
  11. If that's the case then yeah it won't work then. Cause it still gives the errors.
     
  12. Using an async task here probably makes performance even worse if I were to take a guess. Parallel execution isn’t a magic solution to everything.

    This is your issue. Server performance “suffering” because of your code is pure speculation because you don’t know yet. Get rid of the async tasks and actually run your code. Use a profiler like timings or VisualVM to understand the impact of your code before you optimize it. It helps if you can pinpoint why and where your code is slow, if it even has a significant impact in the first place (because it may well not).
     
  13. I've tested it, however this was just with me online. Thinking about how laggy it'll be when ther're multiple people playing is what worries me.
     
  14. Well how many blocks are we talking about?
     
  15. Well, when I code Detonate (Essentially Explosive) it can be up to 125. (This will be chance based - it won't activate every time you break a block. Excavate can mine break up to 25 and drill up to 20(?)
     
  16. FrostedSnowman

    Resource Staff

    Improvements:
    • Your entire enchantment system seems really flimsy and unstructured, really apply some OOP principles to get the best out of your program. Enchant objects with some kind of Enchant registry.. etc.
    • Use an EnumSet instead of a List, Set::add will be much faster
    • Cache World::getBlockAt and use that Block instance instead of calling it multiple times
    • Unnecessary scheduling going on
    • A lot of repetitive code, look into DRY principle

    If you're worried about any kind of lag that could occur from hundreds of blocks being broken by say 20~+ players each, you could try setting blocks through NMS and avoid the bukkit layer, ultimately producing better performance and speed if done correctly
     
    • Agree Agree x 1
  17. Same as FrostedSnowman.
    You should create some chain of responsibility for your enchantments.
    Create a CustomEnchantment class and hand over the event with an abstract method #handle(BlockBreakEvent)

    If you have problems with a ton of world tasks like breaking blocks then split the work into smaller loads and spread them over multiple ticks.
    You can create a task that works on a queue every tick and breaks/places blocks for 1ms then just stops and lets the next tick handle more blocks.

    Then you just schedule the blocks you want to have broken and wait.
     
  18. I'll look into generally improving it later on.
     
  19. Please do not suggest code with horrendous implementation like that, good lord this is - in great likeliness - slower then actually iterating through the methods. You generate a huge overhead by queueing up an unreasonable number of tasks like that, people are misunderstanding what it means to run asynchrnous block operations - what you suggested is most definitely not the way to do it.

    When you run a block operation asynchronously you attempt to run the geometric modifications off thread while the block changes are applied on the main thread. There are ways to actually modify a world asynchrnously and securely, which I will also elaborate further below.

    The easy way of doing this would be running complex mathemathics online. For example, you could get rid of the need to process arithmetics on the thread. Even something like 1+1 will add up if you need to worry about too many such operations being processed. Also stuff like square roots, exponents, processing certain rules, working with randomness etc can be worked off async, afterwards you just pass one collection to your main thread & have it work off there.

    Do NOT queue one task for one operation, inserting tasks is a thread blocking operation. Besides that you are generating a massive memory overhead and the expenditure of inserting a task is not as small as many people believe. Its acceptable if you have a very small amount of operations to worry about, but its ridiclous to do that with a player.

    Use a Queue implementation (LinkedList maybe) to acquire the maximum performance, index based look up is much slower then a reference chain like used by linked lists.

    Regarding actual asynchronous modification, you can check out the nms implementation of chunks. There is a private field which represents the chunk sections, at the expenditure of one reflective incovation (to grab the array) you can swap out the sections on the main thread while you prepared the type of chunk sections you wanted off thread. This is less suitable for smaller operations like yours though, however it does not matter if you change one block or 65k blocks the main thread expenditure remains the same. You need to process some additional methods too, but everything is within the nms class for chunks. I got it working for myself, I will not share sources on this.

    Anyway, your case would work optimally by preparing a Queue which knows the location and the target block type (Probably use a pair) so that the only thing you need to do on your main thread is applying the blocks. This will be faster because you lack the whole clutter around it that would block the main thread. The chunk section approach is not worthwhile for you because you have too little block operations running, it sounds very likely that the one reflective invocation will consume more resources then a few dozen normal block changes.
     
    • Like Like x 1
  20. While I agree with your post, just to cover all bases and hopefully avoid confusion: 1 + 1 is a constant expression and Java will be able to optimize such a call to 2. So, the computation of 1 + 1 is not done during runtime, but actually compile time. Broadly speaking: a million 1 + 1 operations take the same amount of time when running as a million calls to 2.