Repository: microsoft/terminal
File: src/buffer/out/textBufferCellIterator.cpp (C++)

Code before:
           _attrIter == it._attrIter;
}

// Routine Description:
// - Compares two iterators to see if they're pointing to the different positions in the same buffer or different buffers entirely.
// Arguments:
// - it - The other iterator to compare to this one.
// Return Value:
// - True if it's the same text buffer and different cell position or if they're different buffers. False otherwise.
bool TextBufferCellIterator::operator!=(const TextBufferCellIterator& it) const noexcept
{
    return !(*this == it);
}

// Routine Description:
// - Advances the iterator forward relative to the underlying text buffer by the specified movement
// Arguments:
// - movement - Magnitude and direction of movement.
// Return Value:
// - Reference to self after movement.
TextBufferCellIterator& TextBufferCellIterator::operator+=(const ptrdiff_t& movement)
{
    ptrdiff_t move = movement;
    if (move < 0)
    {
        return (*this) -= (-move);
    }

    auto oldX = _pos.X;
    auto newX = oldX;
    auto newY = _pos.Y;
    bool yChanged = false;
    const auto boundsRightInclusive = _bounds.RightInclusive();
    const auto boundsLeft = _bounds.Left();
    const auto boundsBottomInclusive = _bounds.BottomInclusive();
    const auto boundsTop = _bounds.Top();
    while (move > 0)
    {
        if (newX == boundsRightInclusive)
        {
            newX = boundsLeft;
            newY++;
            yChanged = true;
            if (newY > boundsBottomInclusive)
            {
                newY = boundsTop;
                _exceeded = true;
                break;
            }
        }
        else

Review comment (performance):
Let's say we are doing the "1 million yes" benchmark with a typical 120x80 windows. The `+=` operator will execute 120 x 1000000 = 120000000 times. In this particular method, I think it's safe to say that *every single instruction counts*. 

First, this line here is the early branching to avoid the very rare case (move < 0) and leave it to `-=` operator. Next I replace the `IncrementInBounds` with an simplified,  cache-friendly version, without compromising functionally. And finally I replace the `_GenerateView` with fine-grained `Update` methods.

Code after:
           _attrIter == it._attrIter;
}

// Routine Description:
// - Compares two iterators to see if they're pointing to the different positions in the same buffer or different buffers entirely.
// Arguments:
// - it - The other iterator to compare to this one.
// Return Value:
// - True if it's the same text buffer and different cell position or if they're different buffers. False otherwise.
bool TextBufferCellIterator::operator!=(const TextBufferCellIterator& it) const noexcept
{
    return !(*this == it);
}

// Routine Description:
// - Advances the iterator forward relative to the underlying text buffer by the specified movement
// Arguments:
// - movement - Magnitude and direction of movement.
// Return Value:
// - Reference to self after movement.
TextBufferCellIterator& TextBufferCellIterator::operator+=(const ptrdiff_t& movement)
{
    // Note that this method is called intensively when the terminal is under heavy load.
    // We need to aggressively optimize it, comparing to the -= operator.
    ptrdiff_t move = movement;
    if (move < 0)
    {
        // Early branching to leave the rare case to -= operator.
        // This helps reducing the instruction count within this method, which is good for instruction cache.
        return *this -= -move;
    }

    // The remaining code in this function is functionally equivalent to:
    //   auto newPos = _pos;
    //   while (move > 0 && !_exceeded)
    //   {
    //       _exceeded = !_bounds.IncrementInBounds(newPos);
    //       move--;
    //   }
    //   _SetPos(newPos);
    //
    // _SetPos() necessitates calling _GenerateView() and thus the construction
    // of a new OutputCellView(). This has a high performance impact (ICache spill?).
    // The code below inlines _bounds.IncrementInBounds as well as SetPos.
    // In the hot path (_pos.Y doesn't change) we modify the _view directly.

    // Hoist these integers which will be used frequently later.
    const auto boundsRightInclusive = _bounds.RightInclusive();
    const auto boundsLeft = _bounds.Left();
    const auto boundsBottomInclusive = _bounds.BottomInclusive();
    const auto boundsTop = _bounds.Top();