Skip to content

Improve performance of Symbol concatenation#41992

Merged
KristofferC merged 2 commits intoJuliaLang:masterfrom
BioTurboNick:patch-6
Aug 26, 2021
Merged

Improve performance of Symbol concatenation#41992
KristofferC merged 2 commits intoJuliaLang:masterfrom
BioTurboNick:patch-6

Conversation

@BioTurboNick
Copy link
Copy Markdown
Contributor

# current: Symbol(x...) = Symbol(string(x...))
@btime Symbol(:test, :me)
  111.024 ns (3 allocations: 176 bytes)

Symbol(x::Symbol, y::Symbol) = Symbol(string(x), string(y))
@btime Symbol(:test, :me)
  84.826 ns (3 allocations: 96 bytes) - 25% better in time, 45% better in memory space

This concatenation method is used a lot in Plots.jl and has a material impact on performance: JuliaPlots/Plots.jl#3763

Copy link
Copy Markdown
Member

@oscardssmith oscardssmith left a comment

Choose a reason for hiding this comment

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

Looks good.

@mbauman
Copy link
Copy Markdown
Member

mbauman commented Aug 24, 2021

Seems like we should be able to do even better by going deeper. I seem to have a slower system than you:

Before:

julia> @btime Symbol(:test, :me)
  152.419 ns (3 allocations: 160 bytes)
:testme
lower level hackery
julia> @eval Base begin
       @inline function __unsafe_string!(out, s::Symbol, offs::Integer)
           n = sizeof(s)
           GC.@preserve s out unsafe_copyto!(pointer(out, offs), unsafe_convert(Ptr{UInt8},s), n)
           return n
       end

       function string(a::Union{Char, String, SubString{String}, Symbol}...)
           n = 0
           for v in a
               if v isa Char
                   n += ncodeunits(v)
               else
                   n += sizeof(v)
               end
           end
           out = _string_n(n)
           offs = 1
           for v in a
               offs += __unsafe_string!(out, v, offs)
           end
           return out
       end
       end
string (generic function with 21 methods)

After:

julia> @btime Symbol(:test, :me)
  119.850 ns (1 allocation: 32 bytes)
:testme

Similar ratio in time improvement, but 2 fewer allocations, 1/5th the space, and hugely more versatile.

@BioTurboNick
Copy link
Copy Markdown
Contributor Author

Oh nice, so shall I change to the low-level hackery? I can add some tests to make sure nothing breaks.

@mbauman
Copy link
Copy Markdown
Member

mbauman commented Aug 25, 2021

I just pushed it — I already had it locally. It'd be good to ensure we have testing that includes heterogeneous combinations of a Symbol or two with chars and strings; if you could add that or verify it it'd be great!

@mbauman mbauman changed the title Improve performance of 2-arg Symbol concatenation Improve performance of Symbol concatenation Aug 25, 2021
@BioTurboNick
Copy link
Copy Markdown
Contributor Author

I took a look and I think the existing tests seem comprehensive enough?

@testset "Symbol and gensym" begin
    @test Symbol("asdf") === :asdf
    @test Symbol(:abc,"def",'g',"hi",0) === :abcdefghi0
    @test :a < :b
    @test startswith(string(gensym("asdf")),"##asdf#")
    @test gensym("asdf") != gensym("asdf")
    @test gensym() != gensym()
    @test startswith(string(gensym()),"##")
    @test_throws ArgumentError Symbol("ab\0")
    @test_throws ArgumentError gensym("ab\0")
end

Maybe the only thing useful to add would be empties?

@test Symbol("", :"") == :""
@test Symbol(:"", :"") == :""
@test Symbol(a, :"") == :a
@test Symbol(:"", a) == :a

@KristofferC KristofferC merged commit f1b5abc into JuliaLang:master Aug 26, 2021
@KristofferC KristofferC added the performance Must go faster label Aug 26, 2021
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance Must go faster

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants