Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Use for rather than foreach on List#1523

Merged
davidfowl merged 1 commit into
aspnet:devfrom
benaadams:write-req
Mar 21, 2017
Merged

Use for rather than foreach on List#1523
davidfowl merged 1 commit into
aspnet:devfrom
benaadams:write-req

Conversation

@benaadams
Copy link
Copy Markdown
Contributor

@benaadams benaadams commented Mar 20, 2017

List enumerator is full fat
@rynowak
Copy link
Copy Markdown
Member

rynowak commented Mar 20, 2017

Love it

private void UnpinGcHandles()
{
foreach (var pin in req._pins)
var pinList = _pins;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why the local?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

so its operating on a local register pointer of the List rather than operating on memory location

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Might not be needed (i.e. might put it in register itself) - will check

@benaadams
Copy link
Copy Markdown
Contributor Author

benaadams commented Mar 21, 2017

@rynowak List Add dotnet/coreclr#9539 and Clear+Remove (for valuetypes without refs) dotnet/coreclr#9540 have also been improved in 2.0

@rynowak
Copy link
Copy Markdown
Member

rynowak commented Mar 21, 2017

@benaadams - you're my hero. I've seen these show up in MVC/Routing profiles in the past as well

@benaadams
Copy link
Copy Markdown
Contributor Author

Also @halter73 hopefully most of the GCHandle calls should "disappear" in 2.0 dotnet/coreclr#9473

Copy link
Copy Markdown
Member

@davidfowl davidfowl left a comment

Choose a reason for hiding this comment

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

LGTM

@benaadams
Copy link
Copy Markdown
Contributor Author

@halter73

with local

            var pinList = _pins;
00007FFF08A2B332  push        rbp  
00007FFF08A2B333  push        rbx  
00007FFF08A2B334  sub         rsp,48h  
00007FFF08A2B338  mov         rsi,rcx  
00007FFF08A2B33B  lea         rdi,[rsp+28h]  
00007FFF08A2B340  mov         ecx,8  
00007FFF08A2B345  xor         eax,eax  
00007FFF08A2B347  rep stos    dword ptr [rdi]  
00007FFF08A2B349  mov         rcx,rsi  
00007FFF08A2B34C  mov         rsi,rcx  
00007FFF08A2B34F  mov         rdi,qword ptr [rsi+50h]  
00007FFF08A2B353  mov         ebx,dword ptr [rdi+18h]  
            for (var i = 0; i < count; i++)
00007FFF08A2B356  xor         ebp,ebp  
00007FFF08A2B358  test        ebx,ebx  
00007FFF08A2B35A  jle         00007FFF08A2B390  
            {
                pinList[i].Free();
00007FFF08A2B35C  cmp         ebp,dword ptr [rdi+18h]  
00007FFF08A2B35F  jb          00007FFF08A2B366  
00007FFF08A2B361  call        00007FFF63DE8750  
00007FFF08A2B366  mov         rcx,qword ptr [rdi+8]  
00007FFF08A2B36A  cmp         ebp,dword ptr [rcx+8]  
00007FFF08A2B36D  jae         00007FFF08A2B3F9  
00007FFF08A2B373  movsxd      rax,ebp  
00007FFF08A2B376  mov         rcx,qword ptr [rcx+rax*8+10h]  
00007FFF08A2B37B  mov         qword ptr [rsp+40h],rcx  
00007FFF08A2B380  lea         rcx,[rsp+40h]  
00007FFF08A2B385  call        00007FFF63DDF080  
            for (var i = 0; i < count; i++)
00007FFF08A2B38A  inc         ebp  
00007FFF08A2B38C  cmp         ebp,ebx  
00007FFF08A2B38E  jl          00007FFF08A2B35C  
            }
            pinList.Clear();
00007FFF08A2B390  mov         rcx,rdi  
00007FFF08A2B393  call        00007FFF63FA8440  

            var handleList = _handles;
00007FFF08A2B398  mov         rsi,qword ptr [rsi+58h]  
00007FFF08A2B39C  mov         ebx,dword ptr [rsi+18h]  
            for (var i = 0; i < count; i++)
00007FFF08A2B39F  xor         edi,edi  
00007FFF08A2B3A1  test        ebx,ebx  
00007FFF08A2B3A3  jle         00007FFF08A2B3E7  
            {
                handleList[i].Free();
00007FFF08A2B3A5  cmp         edi,dword ptr [rsi+18h]  
00007FFF08A2B3A8  jb          00007FFF08A2B3AF  
00007FFF08A2B3AA  call        00007FFF63DE8750  
00007FFF08A2B3AF  mov         rcx,qword ptr [rsi+8]  
00007FFF08A2B3B3  cmp         edi,dword ptr [rcx+8]  
00007FFF08A2B3B6  jae         00007FFF08A2B3F9  
00007FFF08A2B3B8  movsxd      rax,edi  
00007FFF08A2B3BB  lea         rax,[rax+rax*2]  
00007FFF08A2B3BF  lea         rcx,[rcx+rax*8+10h]  
00007FFF08A2B3C4  movdqu      xmm0,xmmword ptr [rcx]  
00007FFF08A2B3C8  movdqu      xmmword ptr [rsp+28h],xmm0  
00007FFF08A2B3CE  mov         rax,qword ptr [rcx+10h]  
00007FFF08A2B3D2  mov         qword ptr [rsp+38h],rax  
00007FFF08A2B3D7  lea         rcx,[rsp+28h]  
00007FFF08A2B3DC  call        00007FFF08A28D38  
            for (var i = 0; i < count; i++)
00007FFF08A2B3E1  inc         edi  
00007FFF08A2B3E3  cmp         edi,ebx  
00007FFF08A2B3E5  jl          00007FFF08A2B3A5  
            }
            handleList.Clear();
00007FFF08A2B3E7  mov         rcx,rsi  
00007FFF08A2B3EA  call        00007FFF08A2A008  
00007FFF08A2B3EF  nop  
00007FFF08A2B3F0  add         rsp,48h  
00007FFF08A2B3F4  pop         rbx  
00007FFF08A2B3F5  pop         rbp  
00007FFF08A2B3F6  pop         rsi  
00007FFF08A2B3F7  pop         rdi  
00007FFF08A2B3F8  ret  
00007FFF08A2B3F9  call        00007FFF6840DA50  
00007FFF08A2B3FE  int         3  

without local

            var count = _pins.Count;
00007FFF089FB982  push        rbp  
00007FFF089FB983  push        rbx  
00007FFF089FB984  sub         rsp,48h  
00007FFF089FB988  mov         rsi,rcx  
00007FFF089FB98B  lea         rdi,[rsp+28h]  
00007FFF089FB990  mov         ecx,8  
00007FFF089FB995  xor         eax,eax  
00007FFF089FB997  rep stos    dword ptr [rdi]  
00007FFF089FB999  mov         rcx,rsi  
00007FFF089FB99C  mov         rsi,rcx  
00007FFF089FB99F  mov         rax,qword ptr [rsi+50h]  
00007FFF089FB9A3  mov         edi,dword ptr [rax+18h]  
            for (var i = 0; i < count; i++)
00007FFF089FB9A6  xor         ebx,ebx  
00007FFF089FB9A8  test        edi,edi  
00007FFF089FB9AA  jle         00007FFF089FB9E4  
            {
                _pins[i].Free();
00007FFF089FB9AC  mov         rbp,qword ptr [rsi+50h]      ; extra load
00007FFF089FB9B0  cmp         ebx,dword ptr [rbp+18h]  
00007FFF089FB9B3  jb          00007FFF089FB9BA  
00007FFF089FB9B5  call        00007FFF63DE8750  
00007FFF089FB9BA  mov         rcx,qword ptr [rbp+8]  
00007FFF089FB9BE  cmp         ebx,dword ptr [rcx+8]  
00007FFF089FB9C1  jae         00007FFF089FBA57  
00007FFF089FB9C7  movsxd      rax,ebx  
00007FFF089FB9CA  mov         rcx,qword ptr [rcx+rax*8+10h]  
00007FFF089FB9CF  mov         qword ptr [rsp+40h],rcx  
00007FFF089FB9D4  lea         rcx,[rsp+40h]  
00007FFF089FB9D9  call        00007FFF63DDF080  
            for (var i = 0; i < count; i++)
00007FFF089FB9DE  inc         ebx  
00007FFF089FB9E0  cmp         ebx,edi  
00007FFF089FB9E2  jl          00007FFF089FB9AC  
            }
            _pins.Clear();
00007FFF089FB9E4  mov         rcx,qword ptr [rsi+50h]        ; extra load
00007FFF089FB9E8  cmp         dword ptr [rcx],ecx  
00007FFF089FB9EA  call        00007FFF63FA8440  

            count = _handles.Count;
00007FFF089FB9EF  mov         rax,qword ptr [rsi+58h]  
00007FFF089FB9F3  mov         edi,dword ptr [rax+18h]  
            for (var i = 0; i < count; i++)
00007FFF089FB9F6  xor         ebx,ebx  
00007FFF089FB9F8  test        edi,edi  
00007FFF089FB9FA  jle         00007FFF089FBA42  
            {
                _handles[i].Free();
00007FFF089FB9FC  mov         rbp,qword ptr [rsi+58h]        ; extra load 
00007FFF089FBA00  cmp         ebx,dword ptr [rbp+18h]  
00007FFF089FBA03  jb          00007FFF089FBA0A  
00007FFF089FBA05  call        00007FFF63DE8750  
00007FFF089FBA0A  mov         rcx,qword ptr [rbp+8]  
00007FFF089FBA0E  cmp         ebx,dword ptr [rcx+8]  
00007FFF089FBA11  jae         00007FFF089FBA57  
00007FFF089FBA13  movsxd      rax,ebx  
00007FFF089FBA16  lea         rax,[rax+rax*2]  
00007FFF089FBA1A  lea         rcx,[rcx+rax*8+10h]  
00007FFF089FBA1F  movdqu      xmm0,xmmword ptr [rcx]  
00007FFF089FBA23  movdqu      xmmword ptr [rsp+28h],xmm0  
00007FFF089FBA29  mov         rax,qword ptr [rcx+10h]  
00007FFF089FBA2D  mov         qword ptr [rsp+38h],rax  
00007FFF089FBA32  lea         rcx,[rsp+28h]  
00007FFF089FBA37  call        00007FFF089F8BA8  
            for (var i = 0; i < count; i++)
00007FFF089FBA3C  inc         ebx  
00007FFF089FBA3E  cmp         ebx,edi  
00007FFF089FBA40  jl          00007FFF089FB9FC  
            }
            _handles.Clear();
00007FFF089FBA42  mov         rcx,qword ptr [rsi+58h]         ; extra load
00007FFF089FBA46  cmp         dword ptr [rcx],ecx  
00007FFF089FBA48  call        00007FFF089F9B78  
00007FFF089FBA4D  nop  
00007FFF089FBA4E  add         rsp,48h  
00007FFF089FBA52  pop         rbx  
00007FFF089FBA53  pop         rbp  
00007FFF089FBA54  pop         rsi  
00007FFF089FBA55  pop         rdi  
00007FFF089FBA56  ret  
00007FFF089FBA57  call        00007FFF6840DA50  
00007FFF089FBA5C  int         3  

So extra load per iteration and for clear

@davidfowl davidfowl merged commit 24ed932 into aspnet:dev Mar 21, 2017
@benaadams benaadams deleted the write-req branch March 21, 2017 10:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants