Skip to content

Generate reverse for StaticVector types #802

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

Merged
merged 1 commit into from
Jun 22, 2020

Conversation

OTDE
Copy link
Contributor

@OTDE OTDE commented Jun 20, 2020

Hat tip to @MasonProtter for this neat little trick.

This PR eliminates allocations in SVector containers that hold a single bits eltype and speeds them up by 1000 times the speed of the base branch in some cases:

Benchmarking v = SVector(Tuple(randn(200)))

base branch:

julia> @benchmark reverse($v)
BenchmarkTools.Trial: 
  memory estimate:  486.89 KiB
  allocs estimate:  20699
  --------------
  minimum time:     536.714 μs (0.00% GC)
  median time:      540.254 μs (0.00% GC)
  mean time:        567.529 μs (1.55% GC)
  maximum time:     1.777 ms (65.64% GC)
  --------------
  samples:          8788
  evals/sample:     1

sc/reverse-speedup:

julia> @benchmark reverse($v)
BenchmarkTools.Trial: 
  memory estimate:  0 bytes
  allocs estimate:  0
  --------------
  minimum time:     31.943 ns (0.00% GC)
  median time:      32.063 ns (0.00% GC)
  mean time:        33.041 ns (0.00% GC)
  maximum time:     98.280 ns (0.00% GC)
  --------------
  samples:          10000
  evals/sample:     994

Benchmarking v = SVector(Char.(rand('a':'z', 200))...):

base branch:

julia> @benchmark reverse($v)
BenchmarkTools.Trial: 
  memory estimate:  87.34 KiB
  allocs estimate:  400
  --------------
  minimum time:     462.796 μs (0.00% GC)
  median time:      465.363 μs (0.00% GC)
  mean time:        479.433 μs (0.49% GC)
  maximum time:     2.088 ms (76.58% GC)
  --------------
  samples:          10000
  evals/sample:     1

sc/reverse-speedup:

julia> @benchmark reverse($v)
BenchmarkTools.Trial: 
  memory estimate:  0 bytes
  allocs estimate:  0
  --------------
  minimum time:     10.548 ns (0.00% GC)
  median time:      11.030 ns (0.00% GC)
  mean time:        11.695 ns (0.00% GC)
  maximum time:     54.831 ns (0.00% GC)
  --------------
  samples:          10000
  evals/sample:     999

For non-bits types, the speedup and allocation saves are closer to 100 times those of the base branch:

Benchmarking v = SVector(("a", :b, randn(198)...)) (which has an eltype of Any):

base branch:

julia> @benchmark reverse($v)
BenchmarkTools.Trial: 
  memory estimate:  485.23 KiB
  allocs estimate:  20694
  --------------
  minimum time:     537.686 μs (0.00% GC)
  median time:      552.485 μs (0.00% GC)
  mean time:        591.481 μs (1.78% GC)
  maximum time:     2.932 ms (54.48% GC)
  --------------
  samples:          8438
  evals/sample:     1

sc/reverse-speedup:

julia> @benchmark reverse($v)
BenchmarkTools.Trial: 
  memory estimate:  4.70 KiB
  allocs estimate:  200
  --------------
  minimum time:     5.252 μs (0.00% GC)
  median time:      5.419 μs (0.00% GC)
  mean time:        5.933 μs (1.44% GC)
  maximum time:     167.364 μs (94.68% GC)
  --------------
  samples:          10000
  evals/sample:     6

Benchmark information:

julia> versioninfo()
Julia Version 1.4.1
Commit 381693d3df* (2020-04-14 17:20 UTC)
Platform Info:
  OS: Linux (x86_64-pc-linux-gnu)
  CPU: Intel(R) Core(TM) i7-8700K CPU @ 3.70GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-8.0.1 (ORCJIT, skylake)
Environment:
  JULIA_EDITOR = atom  -a
  JULIA_NUM_THREADS = 6

@OTDE
Copy link
Contributor Author

OTDE commented Jun 20, 2020

CI failures appear to be unrelated.

Copy link
Member

@andyferris andyferris left a comment

Choose a reason for hiding this comment

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

Nice!

I wish the tuple methods in base were a bit more robust, but oh well.

@c42f c42f merged commit 68b10c2 into JuliaArrays:master Jun 22, 2020
@c42f
Copy link
Member

c42f commented Jun 22, 2020

Looks good to me, cheers!

@MasonProtter
Copy link

Any thoughts on if this should be attempted to be upstreamed to Base?

@martinholters
Copy link
Collaborator

Ref. JuliaLang/julia#16604 (note JuliaLang/julia#16604 (comment)).

@andyferris
Copy link
Member

I think you will want if @generated for a Tuple method if you want this accepted in Base. But yes it might be a good idea?

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.

5 participants