Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize dispose logic #48

Merged
merged 9 commits into from
Nov 2, 2021
Merged

Optimize dispose logic #48

merged 9 commits into from
Nov 2, 2021

Conversation

bitfaster
Copy link
Owner

@bitfaster bitfaster commented Jun 29, 2020

Experiment - can JIT elide code like:

if (value is IDisposable d)
{
   d.Dispose();
}

@bitfaster
Copy link
Owner Author

Benchmark is bogus (ran a minute ago and not optimized was fastest):

Method Mean Error StdDev Ratio RatioSD
HandWritten 285.6 ns 2.21 ns 1.96 ns 1.00 0.00
NotOptimized 287.8 ns 3.83 ns 3.58 ns 1.01 0.02
GenericDisposer3 285.9 ns 4.24 ns 3.76 ns 1.00 0.01

{
public void Dispose(T value)
{
if (typeof(T) is IDisposable)
Copy link
Owner Author

@bitfaster bitfaster Oct 26, 2021

Choose a reason for hiding this comment

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

Stupid :) - this is checking if Type is IDisposable, because the result of typeof(T) is an instance of Type.

@bitfaster
Copy link
Owner Author

bitfaster commented Oct 26, 2021

can try using static readonly field - issue 4920, but looks like it doesn't apply to generics.

image

@bitfaster
Copy link
Owner Author

bitfaster commented Oct 31, 2021

Wrapping the standard if value is IDisposable check into a generic is equal speed/code size:

Method Mean Ratio Code Size Allocated
HandWritten 3.527 us 1.00 0 KB 23 KB
NotOptimized 3.505 us 0.99 0 KB 23 KB
GenericDisposerReadonlyProperty 8.293 us 2.36 0 KB 47 KB
GenericDisposerStdCheck 3.502 us 0.99 0 KB 23 KB

And it looks like it does decrease the size of the code by about 4% (from running the LRU bench with DisassemblyDiagnoser):

With Disposer:

Method Code Size
ConcurrentDictionary 340 B
FastConcurrentLru 451 B
ConcurrentLru 473 B
FastConcurrentTLru 638 B
ConcurrentTLru 709 B
ClassicLru 738 B

Original:

Method Code Size
ConcurrentDictionary 340 B
FastConcurrentLru 473 B
ConcurrentLru 495 B
FastConcurrentTLru 666 B
ConcurrentTLru 737 B
ClassicLru 756 B

The actual runtime is pretty much identical - within the variance from running the same thing over and over again.

@bitfaster
Copy link
Owner Author

Generated assembly is the same as hand written code:

; BitFaster.Caching.Benchmarks.DisposerBench.HandWritten()
       push      rsi
       sub       rsp,20
       xor       esi,esi
M00_L00:
       mov       rcx,offset MT_BitFaster.Caching.Benchmarks.Disposable
       call      CORINFO_HELP_NEWSFAST
       cmp       byte ptr [rax+8],0
       jne       short M00_L01
       mov       byte ptr [rax+8],1
M00_L01:
       inc       esi
       cmp       esi,3E8
       jl        short M00_L00
       add       rsp,20
       pop       rsi
       ret
; Total bytes of code 48
; BitFaster.Caching.Benchmarks.DisposerBench.GenericDisposerStdCheck()
       push      rsi
       sub       rsp,20
       xor       esi,esi
M00_L00:
       mov       rcx,offset MT_BitFaster.Caching.Benchmarks.Disposable
       call      CORINFO_HELP_NEWSFAST
       cmp       byte ptr [rax+8],0
       jne       short M00_L01
       mov       byte ptr [rax+8],1
M00_L01:
       inc       esi
       cmp       esi,3E8
       jl        short M00_L00
       add       rsp,20
       pop       rsi
       ret
; Total bytes of code 48

@bitfaster bitfaster merged commit 043b548 into main Nov 2, 2021
@bitfaster bitfaster deleted the users/alexpeck/disposer branch November 2, 2021 01:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant