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

Use a codeunit type-assert that is easier for inference to elide #37192

Merged
merged 1 commit into from
Aug 25, 2020

Conversation

timholy
Copy link
Sponsor Member

@timholy timholy commented Aug 25, 2020

Circumvents #37135

@timholy timholy merged commit dbf5e00 into master Aug 25, 2020
@timholy timholy deleted the teh/string_union branch August 25, 2020 17:31
simeonschaub pushed a commit to simeonschaub/julia that referenced this pull request Aug 29, 2020
@omus
Copy link
Member

omus commented Sep 4, 2020

I was working on a performance improvement for ==(::AbstractString, ::AbstractString) but I discovered this PR addresses the performance issue I was noticing (mostly allocations):

Before this PR (6aee988):

julia> str = "A" ^ 1000;

julia> @btime invoke(==, Tuple{AbstractString, AbstractString}, $"ABC", $"ABC")
  4.357 ns (0 allocations: 0 bytes)
true

julia> @btime $(SubString("ABC")) == $"ABCD"
  29.574 ns (2 allocations: 64 bytes)
false

julia> @btime $(SubString("ABC")) == $"ABC"
  32.492 ns (2 allocations: 64 bytes)
true

julia> @btime $(SubString("ABC")) == $"DEF"
  31.985 ns (2 allocations: 64 bytes)
false

julia> @btime $(SubString(str)) == $str
  65.086 ns (2 allocations: 64 bytes)
true

With this PR (dbf5e00):

julia> str = "A" ^ 1000;

julia> @btime invoke(==, Tuple{AbstractString, AbstractString}, $"ABC", $"ABC")
  3.907 ns (0 allocations: 0 bytes)
true

julia> @btime $(SubString("ABC")) == $"ABCD"
  0.033 ns (0 allocations: 0 bytes)
false

julia> @btime $(SubString("ABC")) == $"ABC"
  4.003 ns (0 allocations: 0 bytes)
true

julia> @btime $(SubString("ABC")) == $"DEF"
  3.282 ns (0 allocations: 0 bytes)
false

julia> @btime $(SubString(str)) == $str
  38.978 ns (0 allocations: 0 bytes)
true

I'll be adding some benchmarks for this.

@KristofferC
Copy link
Sponsor Member

Is this v1.5? Because AFAIU this just fixed an earlier regression on nightly. But good to have benchmarks anyway.

@omus
Copy link
Member

omus commented Sep 8, 2020

I noticed similar slow performance on Julia 1.3.1, 1.4.2, and 1.5.1. All of these versions were substantially worse with the final test:

julia> @btime $(SubString(str)) == $str
  4.043 μs (2 allocations: 112 bytes)
true

@KristofferC
Copy link
Sponsor Member

KristofferC commented Sep 8, 2020

Seems more likely that the improvement you see was because of #35973

@omus
Copy link
Member

omus commented Sep 8, 2020

You are correct that #35973 is the main driver behind the performance improvement. I'll update my benchmarks to reflect this.

@omus
Copy link
Member

omus commented Sep 8, 2020

I've also opened #37467 which improves performance of the fallback ==(::AbstractString, ::AbstractString) call.

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.

None yet

4 participants