Get neighbor from same map block if possible in ABMHandler

[lhofhansl]

I noticed that when I use the Technic mod and a larg'ish active_block_range - 7 or 8 in my case - I see messages like these:
2017-01-05 21:55:04: WARNING[Server]: active block modifiers took 623ms (longer than 200ms)

Without actually doing anything. Turns out that the time is spent in the required neighbor check.
Looking at the code one can observe that the majority of the neighbors of the nodes are actually found in the very map block that we already have a reference to; so use that one to get the neighorhing map node instead of going back to getting the mapnode via map->sector->mapblock->mapnode.

That brought the max down from 623ms to to 223ms. And the average from about 490ms to 210ms.

[lhofhansl]

As an aside, I have to think that there have to be smarter ways to check the neighbors as we sweep through most of them in the outer loop anyway. Any ideas are welcome, until then this is a good stop-gap measure I think.

[lhofhansl]

And note that in the worst case we're checking 16^3 * (3^3-1) = 106496 per single map block, so this is well worth optimizing.

[tenplus1]

+1 on optimizing abm code, but a quick question: If this change has it working on same map block and only one neighbor node is sitting just outside that mapblock, will it still call the abm ? e.g. abm looks for trunks next to leaves and the trunk is in one map block and the leaves are beside it but in another map block...

[lhofhansl]

@tenplus1 yes, it's still checking and triggering on the exact same nodes. Just the way the nodes are returned is faster.

[Zeno-]

+1

[sorcerykid]

I'm really curious if the neighboring nodes are in one or more mapblocks that are not yet loaded, are they all loaded as a result? And do they stay loaded (or cached) for the duration of the loop, or are they repeatedly reloaded for other neighboring nodes? If the latter, then would you say there are any performance penalties?

EDIT: Upon closer inspection, it would seem no mapblocks are loaded or emerged. However, I wonder if further optimizations are still possible to avoid map->sector->mapblock->mapnode trials for all the neighboring nodes that reside in a nearby mapblock.

I supposed one possible solution might be to cache the initial references to the nearby mapblocks in a local lookup table. Would something like this work (please excuse my code, if it's incorrect)?

if (block->isValidPosition(p1)) {
    :
    :
} else {
    MapBlock* block2
    v3s16 blockpos = getNodeBlockPos(p1 + block->getPosRelative());
    std::map<v3s16, MapBlock*>::iterator i = near_blocks.find(blockpos);

    if(i == near_blocks.end())
        // cache pointer to surrounding mapblock for reference during
        // successive iterations
        block2 = map->getBlockNoEx(p1 + block->getPosRelative());
        near_blocks[blockpos] = block2;
    else
        // otherwise get the cached mapblock pointer
        block2 = i->second;

    v3s16 p2 = p1 - block2->getPosRelative();

    const MapNode &n = block2->getNodeUnsafe(p2);
    c = n.getContent();
} 

Of course, near_blocks would need to be declared as std::map<v3s16, MapBlock*> at the beginning of the function scope