# Solved Sphere for-loop optimization

Discussion in 'Spigot Plugin Development' started by Cosmic_luck, Jan 26, 2020.

1. ### Cosmic_luck

I have a working sphere for-loop, and every time a player earns a level, or some other event is called, I take the players home location and create a sphere. In this sphere, any blocks I choose, for example Mycelium, will change into another block, in this case Grass.

What I am trying to do to save on server resources (because trying to loop through a large radius of blocks is taxing), is only check the first layer or two of new blocks, and then skipping everything in the center. So if my sphere has a radius of 10 blocks, and the next time I level up for example, I want it to be a radius of 11 blocks, I already replaced the previous set of blocks, so theres no need to loop through them again.

Code (Java):
public List<Block> sphere(Location center, int radius) {
List<Block> sphere = new ArrayList<Block>();
if (Math.sqrt((X * X) + (Y * Y) + (Z * Z)) <= radius) {
Block block = center.getWorld().getBlockAt(X + center.getBlockX(), Y + center.getBlockY(), Z + center.getBlockZ());
if (block.getType() != Material.AIR && block.getType() == Material.MYCELIUM || block.getType() == Material.OBSIDIAN || block.getType() == Material.NETHERRACK || block.getType() == Material.COARSE_DIRT) {
}
}
//From here, I added test code to try and check each loop if the X, Y, or Z ints had already looped twice, if so add the radius /2 to the int. This "works", however I saw no performance increase, and it is just as laggy as it was without these checks in place
}
}
}
}
}
}
return sphere;
}
Is there anything else I can try to get this to work? I figured skipping the "core" of the sphere would help in performance, but it doesn't seem to have helped at all.

Protip, get rid of the squareroot and use radius * radius, squaring the radius is much faster than square rooting everything else.
Also you might want to look at those new checks, you probably want some inequalities not equalities

• Like x 1
3. ### System

I suggest you spread the work over multiple ticks, to avoid lag spikes.

Also you should move constant values out of the loops so as to calculate them only once, like center.getWorld() and center.getBlockX().
And you could switch over the value of block.getType() instead of using multiple if's:
Code (Java):
switch (block.getType()) {
case MYCELIUM:
case OBSIDIAN:
case NETHERRACK:
case COARSE_DIRT:
}
Also note you had a reduntant check against Material.AIR.

4. ### Warren1001

not how that works. using #getWorld or #getBlockX doesnt create anything. it just references a value. it makes absolutely no difference in terms of performance.

/e might improve code readability tho

There's the potential for a difference but it's not likely to be significant.

6. ### System

How do you know it just references a value? Is it stated in the Javadoc, or are you relying on implementation details to make your statement?
Are you going to examine the source of every method to determine if its result should be cached or not?

7. ### CrypticCabub

I tend to stick to the safe side of getting the data outside the loop as well. Even if it is just a pointer reference, there's no guarantee that the underlying implementation isn't calculating that value on the fly each time (such as a schematic with relative locations to a reference point).

Also as a side note: the idea of "spread the task out over multiple ticks" is exactly what I am referring to in my thread about multi-tick tasks. Spigot does not have any direct support, that I am aware of, for multiple plugins on the server wanting to register multi-tick tasks in this fashion. sync repeating tasks are close, but they force plugins to guess at how much spare time is available each tick to run such tasks. Within the Spigot architecture, would it be possible to expand the scheduler to support some form of deferred task that runs using any spare time remaining at the end of each tick cycle but can be deferred to the next tick if there is not any spare time remaining?

8. ### Warren1001

general convention would state that any simple getter method simply fetches a reference to a value. obviously if such a method has a different name, such as construct, or a getter that takes an argument, then storing your own local reference would be a good idea.

its not as though examining the source of methods is difficult. intellij will decompile any class for you on demand by simply right clicking a method or class or whatever and pressing implementation or declaration

9. ### CrypticCabub

Not so sure that's general convention so much as assumed convention. The outward facing interface of the class uses the getters to get properties of the class, but in some cases those properties may be calculated properties rather than simple references. one example would be an arena configuration that stores spawn locations as vectors relative to the arena's location (I use this for my dynamically generated arenas)

10. ### Schottky

You can use spherical coordinates to reduce the amount of times looped. However; you’d have to compute the Cartesian coordinates from those. You’d probably be faster, if you can use lookup-tables for that.

11. ### drives_a_ford Moderator

A good counter example is ItemStack#getItemMeta. But it's not the only one, obviously.