what's the purpose of a local variable copy of internal array in dotnet collections source code

96 views Asked by At

as dotnet collection Stack.cs source code

public T Peek()
{
    int size = _size - 1;
    T[] array = _array;

    if ((uint)size >= (uint)array.Length)
    {
          ThrowForEmptyStack();
    }

    return array[size];
}

there is a local reference copy of the backing array T[] array = _array; source here

and a lot of similar codes in the Collections codebase.

What's the purpose of this? is it related to thread safety or any other consideration? Why just not use the class field _array instead?

I can't find any clues in google, so expecting some insights here.

2

There are 2 answers

0
Charlieface On BEST ANSWER

It seems to be due to JIT optimization.

While the JIT does have logic to do bounds check elision, in the past it did not work if you refer to the field twice, rather than a local variable.

The function ThrowForEmptyStack is known to be non-returning:

private void ThrowForEmptyStack()
{
    Debug.Assert(_size == 0);
    throw new InvalidOperationException(SR.InvalidOperation_EmptyStack);
}

So when the code does the check

if ((uint)size >= (uint)array.Length)
{
    ThrowForEmptyStack();
}

the JIT now knows that the array index cannot go out-of-bounds, because the bounds check has already been done. So it can elide the normal array bounds check in the generated machine code.

That linked article mentions static fields, not instance fields, so it's not clear, but see this discussion in the runtime repo. So in the past, if you used a field then it can't make that optimization. This seems to have been fixed, so the extra code was later removed.

3
Matthew Watson On

As per the check-in comments for the change that introduced this temporary copy this is nothing to do with thread safety.

Rather, it's to do with optimising the code by removing a bounds check:

With value types the effect is not so big, because there is still one (manual) check for bounds. For reference types one bounds-check can be saved, so there is a win.

The comments and feedback for those changes are quite interesting; there's some discussion about whether the JIT will optimise away the bounds checking or not.