"Lag" when using big region target selectors (dx, dy and dz)

At the first problem, of course Minecraft hangs if you enter a command with a ridiculous area assigned to it. Minecraft will try find all players in all those coordinates possible, in a smaller area this is done within a fraction of a second, here it will trip everything up, resulting in the problem you experience.

Oops, just did a mess... I copied the preset from another bug report and forgot to remove the ending part. I just wanted to point out the region bug.
Anyhow, it still sounds weird that it takes so long. I mean, even if you check for the whole world, 99.99% of the chunks aren't even loaded. I think it would be better either to review the code for the regions or to deny the execution if the selection is too big. I was messing around with that in my vanilla server and it took a while before I understood what happened and restarted the server.

Confirmed in 1.9.1-pre3. The large values for X, Y, and Z aren't needed, the radius argument by itself is enough to cause the lag.

This is most definitely a bug. Since unloaded chunks aren't searched, there is no reason why using a larger search radius should cause lag. No additional chunks are being searched. Additionally, using no search radius doesn't cause any lag at all, even though it is searching the entire world (which is a greater area than part of the world).

Can't reproduce the issue for Player-only selectors (@a,@p,@r without type). Tested only in singleplayer
Relates to MC-54932

Time for some code analysis! This is based on MCP for 1.10.2. Please link this in the description.

So the reason for both this and MC-54932 is the same. As @unknown said, this does not affect Player-only selectors in most cases; if the players make up 1/16 or more of the loaded entities, the bug will occur.

The bottleneck is the method getEntitiesWithinAABB, which is used for both radius- and region-based selection of entities (and players in the 1/16 case mentioned above). It loops through all chunks within the bounding box and, if they are loaded, adds any of their entities that fall within the bounding box.

public <T extends Entity> List<T> getEntitiesWithinAABB(Class <? extends T > clazz, AxisAlignedBB aabb, @Nullable Predicate <? super T > filter)
{
    int i = MathHelper.floor_double((aabb.minX - MAX_ENTITY_RADIUS) / 16.0D);
    int j = MathHelper.ceiling_double_int((aabb.maxX + MAX_ENTITY_RADIUS) / 16.0D);
    int k = MathHelper.floor_double((aabb.minZ - MAX_ENTITY_RADIUS) / 16.0D);
    int l = MathHelper.ceiling_double_int((aabb.maxZ + MAX_ENTITY_RADIUS) / 16.0D);
    List<T> list = Lists.<T>newArrayList();

    for (int i1 = i; i1 < j; ++i1)
    {
        for (int j1 = k; j1 < l; ++j1)
        {
            if (this.isChunkLoaded(i1, j1, true))
            {
                this.getChunkFromChunkCoords(i1, j1).getEntitiesOfTypeWithinAAAB(clazz, aabb, list, filter);
            }
        }
    }

    return list;
}

To solve the bug, the method must be able to quickly determine whether it will be more efficient to loop through all possible chunks in the area (and then check if they are loaded) or through all loaded chunks (and then check if they are in the area), and then to loop through the loaded chunks if that is the decision. Therefore I suggest adding two methods to IChunkProvider: getNumLoadedChunks() and iterLoadedChunks(), both of which should be trivial to implement.

The new method:

public <T extends Entity> List<T> getEntitiesWithinAABB(Class <? extends T > clazz, AxisAlignedBB aabb, @Nullable Predicate <? super T > filter)
{
    int i = MathHelper.floor_double((aabb.minX - MAX_ENTITY_RADIUS) / 16.0D);
    int j = MathHelper.ceiling_double_int((aabb.maxX + MAX_ENTITY_RADIUS) / 16.0D);
    int k = MathHelper.floor_double((aabb.minZ - MAX_ENTITY_RADIUS) / 16.0D);
    int l = MathHelper.ceiling_double_int((aabb.maxZ + MAX_ENTITY_RADIUS) / 16.0D);
    List<T> list = Lists.<T>newArrayList();
    
    int chunksInAABB = (j - i) * (l - k);
    
    if (chunksInAABB > this.chunkProvider.getNumLoadedChunks())
    {
        for (Chunk chunk : this.chunkProvider.iterLoadedChunks())
        {
            if (chunk.xPosition >= i && chunk.xPosition < j && chunk.zPosition >= k && chunk.zPosition < l)
            {
                chunk.getEntitiesOfTypeWithinAAAB(clazz, aabb, list, filter);
            }
        }
    }
    else
    {
        for (int i1 = i; i1 < j; ++i1)
        {
            for (int j1 = k; j1 < l; ++j1)
            {
                if (this.isChunkLoaded(i1, j1, true))
                {
                    this.getChunkFromChunkCoords(i1, j1).getEntitiesOfTypeWithinAAAB(clazz, aabb, list, filter);
                }
            }
        }
    }

    return list;
}

This way the function will have exactly the same input and output, but will never check more chunks than are actually loaded.

Can confirm for 1.16 pre release 5

Can confirm for 1.16.1

can confirm for 20w27a

Can confirm in 20w48a.

For me, this was fixed somewhere around 1.17. Is anyone still experiencing this issue?