Skip to content

Conversation

vchuravy
Copy link
Member

@vchuravy vchuravy commented Nov 15, 2023

@ccall jl_method_lookup_by_tt(Tuple{typeof(+), Int, Int}::Any, Base.get_world_counter()::Csize_t)::Ref{Core.MethodInstance}

Ref JuliaGPU/GPUCompiler.jl#530 (comment)

@maleadt
Copy link
Member

maleadt commented Nov 15, 2023

Performance of the slow path seems good enough for GPUCompiler?

julia> using BenchmarkTools

julia> @benchmark @ccall jl_method_lookup_by_tt(Tuple{typeof(identity), Nothing}::Any, Base.get_world_counter()::Csize_t)::Ref{Core.MethodInstance}
BenchmarkTools.Trial: 10000 samples with 991 evaluations.
 Range (min  max):  40.202 ns  63.825 ns  ┊ GC (min  max): 0.00%  0.00%
 Time  (median):     41.979 ns              ┊ GC (median):    0.00%
 Time  (mean ± σ):   41.988 ns ±  0.638 ns  ┊ GC (mean ± σ):  0.00% ± 0.00%

             ▁  ▂▂▁▂▃▁    ▂▅▇██▅▄▃▁                           ▂
  ▆▇▁▅▅▅▇▆▄▅▇█▆███████▇▆▇████████████▇▆▅▁▃▃▁▃▁▁▃▁▁▃▁▃▁▁▁▁▃▃▅▇ █
  40.2 ns      Histogram: log(frequency) by time        44 ns <

 Memory estimate: 0 bytes, allocs estimate: 0.

julia> @benchmark @ccall jl_method_lookup(Any[identity, nothing]::Ptr{Any}, 2::Csize_t, Base.get_world_counter()::Csize_t)::Ref{Core.MethodInstance}
BenchmarkTools.Trial: 10000 samples with 992 evaluations.
 Range (min  max):  38.800 ns   15.855 μs  ┊ GC (min  max): 0.00%  99.62%
 Time  (median):     41.764 ns               ┊ GC (median):    0.00%
 Time  (mean ± σ):   46.033 ns ± 172.107 ns  ┊ GC (mean ± σ):  5.05% ±  1.69%

         ▆▇▅         ▁█
  ▂▂▃▃▃▂▄███▇▄▂▂▂▁▂▂▃███▃▂▂▂▂▂▂▂▂▂▂▂▁▁▁▂▁▂▂▁▁▂▁▂▂▂▁▂▂▂▂▂▂▂▂▂▂▂ ▃
  38.8 ns         Histogram: frequency by time         56.8 ns <

 Memory estimate: 48 bytes, allocs estimate: 1.

julia> @benchmark @ccall jl_apply_generic(identity::Any, Any[nothing]::Ptr{Any}, 1::Csize_t)::Any
BenchmarkTools.Trial: 10000 samples with 995 evaluations.
 Range (min  max):  29.116 ns   9.606 μs  ┊ GC (min  max): 0.00%  99.47%
 Time  (median):     32.322 ns              ┊ GC (median):    0.00%
 Time  (mean ± σ):   32.950 ns ± 95.852 ns  ┊ GC (mean ± σ):  2.90% ±  0.99%

          ▅█       ▂▃▅▄▁                    ▂▃▃▃▆▄▄▃▅▂▂
  ▁▁▂▂▂▄▄▆██▆▅▃▃▄▄▆█████▇▄▃▃▂▂▂▁▁▁▁▂▃▃▆▇▆▆▆█████████████▇▄▂▂▂ ▄
  29.1 ns         Histogram: frequency by time        33.9 ns <

 Memory estimate: 32 bytes, allocs estimate: 1.

@vchuravy vchuravy added the needs tests Unit tests are required for this change label Nov 15, 2023
@vchuravy vchuravy removed the needs tests Unit tests are required for this change label Nov 24, 2023
@vchuravy vchuravy marked this pull request as ready for review November 24, 2023 20:23
@vchuravy vchuravy force-pushed the vc/lookup_generic branch 2 times, most recently from 97220a0 to a3438ff Compare November 25, 2023 17:09
@vchuravy vchuravy requested a review from vtjnash December 14, 2023 13:47
@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Dec 14, 2023
@vchuravy vchuravy merged commit b57f8d1 into master Dec 18, 2023
@vchuravy vchuravy deleted the vc/lookup_generic branch December 18, 2023 09:20
@DilumAluthge DilumAluthge removed the merge me PR is reviewed. Merge when all tests are passing label Dec 21, 2023
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.

4 participants