Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Queue<T> optimization of (Try)Dequeue #26087

Merged
stephentoub merged 3 commits into from
Jan 30, 2018
Merged

Queue<T> optimization of (Try)Dequeue #26087

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bd9c7da
Queue RCE
gfoidl Dec 28, 2017
2964c4d
MoveNext uses temp-Variable for register access instead of memory access
gfoidl Dec 28, 2017
244839c
Addressed PR feedback
gfoidl Jan 29, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions src/System.Collections/src/System/Collections/Generic/Queue.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,15 +234,18 @@ IEnumerator IEnumerable.GetEnumerator()
// InvalidOperationException.
public T Dequeue()
{
int head = _head;
T[] array = _array;

if (_size == 0)
{
ThrowForEmptyQueue();
}

T removed = _array[_head];
T removed = array[head];
if (RuntimeHelpers.IsReferenceOrContainsReferences<T>())
{
_array[_head] = default(T);
array[head] = default;
}
MoveNext(ref _head);
_size--;
Expand All @@ -252,16 +255,19 @@ public T Dequeue()

public bool TryDequeue(out T result)
{
int head = _head;
T[] array = _array;

if (_size == 0)
{
result = default(T);
result = default;
return false;
}

result = _array[_head];
result = array[head];
if (RuntimeHelpers.IsReferenceOrContainsReferences<T>())
{
_array[_head] = default(T);
array[head] = default;
}
MoveNext(ref _head);
_size--;
Expand Down Expand Up @@ -367,10 +373,15 @@ private void SetCapacity(int capacity)
// Increments the index wrapping it if necessary.
private void MoveNext(ref int index)
{
// It is tempting to use the remainder operator here but it is actually much slower
// than a simple comparison and a rarely taken branch.
// It is tempting to use the remainder operator here but it is actually much slower
// than a simple comparison and a rarely taken branch.
// JIT produces better code than with ternary operator ?:
int tmp = index + 1;
index = (tmp == _array.Length) ? 0 : tmp;
if (tmp == _array.Length)
{
tmp = 0;
}
index = tmp;
Copy link
Copy Markdown
Member

@stephentoub stephentoub Jan 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AndyAyersMS, is there a good reason for the difference in codegen here? It's unfortunate if the more concise / arguably simpler form results in worse code. (That said, I'm only basing the "JIT produces better code" on the comment.)

Copy link
Copy Markdown
Member Author

@gfoidl gfoidl Jan 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for reference:

public class Program
{
    private static int[] _array = new int[3];

    static Program() {}

    public static void Main(string[] args)
    {
        int index = 3;
        MoveNext1(ref index);
        MoveNext2(ref index);
    }

    private static void MoveNext1(ref int index)
    {
        int tmp = index + 1;
        index = (tmp == _array.Length) ? 0 : tmp;
    }

    private static void MoveNext2(ref int index)
    {
        int tmp = index + 1;
        if (tmp == _array.Length) tmp = 0;

        index = tmp;
    }
}
; Assembly listing for method ConsoleApplication.Program:MoveNext1(byref)
; Emitting BLENDED_CODE for X64 CPU with AVX
; optimized code
; rbp based frame
; partially interruptible
; Final local variable assignments
;
;  V00 arg0         [V00,T00] (  4,  4   )   byref  ->  rdi        
;  V01 loc0         [V01,T01] (  3,  2.50)     int  ->  rax        
;  V02 tmp0         [V02,T02] (  3,  2   )   byref  ->  rdi        
;  V03 tmp1         [V03,T03] (  3,  2   )   byref  ->  rdi        
;  V04 tmp2         [V04,T04] (  3,  2   )     int  ->  rax        
;# V05 OutArgs      [V05    ] (  1,  1   )  lclBlk ( 0) [rsp+0x00]  
;
; Lcl frame size = 0

G_M9976_IG01:
       55                   push     rbp
       488BEC               mov      rbp, rsp

G_M9976_IG02:
       8B07                 mov      eax, dword ptr [rdi]
       FFC0                 inc      eax
       48BEB8070040367F0000 mov      rsi, 0x7F36400007B8
       488B36               mov      rsi, gword ptr [rsi]
       394608               cmp      dword ptr [rsi+8], eax
       7402                 je       SHORT G_M9976_IG03
       EB02                 jmp      SHORT G_M9976_IG04

G_M9976_IG03:
       33C0                 xor      eax, eax

G_M9976_IG04:
       8907                 mov      dword ptr [rdi], eax

G_M9976_IG05:
       5D                   pop      rbp
       C3                   ret      

; Total bytes of code 34, prolog size 4 for method ConsoleApplication.Program:MoveNext1(byref)
; ============================================================
; Assembly listing for method ConsoleApplication.Program:MoveNext2(byref)
; Emitting BLENDED_CODE for X64 CPU with AVX
; optimized code
; rsp based frame
; partially interruptible
; Final local variable assignments
;
;  V00 arg0         [V00,T00] (  4,  4   )   byref  ->  rdi        
;  V01 loc0         [V01,T01] (  4,  3.50)     int  ->  rax        
;# V02 OutArgs      [V02    ] (  1,  1   )  lclBlk ( 0) [rsp+0x00]  
;
; Lcl frame size = 0

G_M9979_IG01:

G_M9979_IG02:
       8B07                 mov      eax, dword ptr [rdi]
       FFC0                 inc      eax
       48BEB8070040367F0000 mov      rsi, 0x7F36400007B8
       488B36               mov      rsi, gword ptr [rsi]
       394608               cmp      dword ptr [rsi+8], eax
       7502                 jne      SHORT G_M9979_IG03
       33C0                 xor      eax, eax

G_M9979_IG03:
       8907                 mov      dword ptr [rdi], eax

G_M9979_IG04:
       C3                   ret      

; Total bytes of code 27, prolog size 0 for method ConsoleApplication.Program:MoveNext2(byref)
; ============================================================

Copy link
Copy Markdown
Member

@AndyAyersMS AndyAyersMS Jan 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for noticing this. Looks like (based on the above) there is a pattern that the jit's flow optimizer doesn't spot that leads to a bit of extra branching.

On Windows the codegen for MoveNext1 has an extra mov thrown in just because:

G_M38324_IG02:
       8B01                 mov      eax, dword ptr [rcx]
       FFC0                 inc      eax
       488BD1               mov      rdx, rcx
       48B9E028009012020000 mov      rcx, 0x212900028E0
       488B09               mov      rcx, gword ptr [rcx]
       394108               cmp      dword ptr [rcx+8], eax
       7402                 je       SHORT G_M38324_IG03
       EB02                 jmp      SHORT G_M38324_IG04

G_M38324_IG03:
       33C0                 xor      eax, eax

G_M38324_IG04:
       8902                 mov      dword ptr [rdx], eax

G_M38324_IG05:
       C3                   ret

Opened dotnet/coreclr#16079.

}

private void ThrowForEmptyQueue()
Expand Down