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

improve performance of parse #28787

Merged
merged 1 commit into from
Aug 22, 2018
Merged

improve performance of parse #28787

merged 1 commit into from
Aug 22, 2018

Conversation

KristofferC
Copy link
Sponsor Member

Improves performance by preventing everything getting lifted to the Char domain.

Benchmark

function parseintperf(t)
   local n, m
   for i=1:t
       n = rand(UInt32)
       s = string(n, base = 16)
       m = UInt32(parse(Int64,s, base = 16))
       @assert m == n
   end
   return n
end


strings = [string(rand(UInt32)) for i in 1:10^4];
f(strings) = for s in strings; parse(Int, s) end;

Before

julia> @btime parseintperf(1000);
  189.958 μs (2999 allocations: 109.36 KiB)

julia> @btime f(strings)
  848.226 μs (0 allocations: 0 bytes)

After

julia> @btime parseintperf(1000)
  170.692 μs (2999 allocations: 109.36 KiB)

julia> @btime f(strings)
  709.092 μs (0 allocations: 0 bytes)

Still not completely caught up with 0.6 for parseintperf but getting close.

@MasonProtter
Copy link
Contributor

What were the numbers for 0.6, for comparison?

@StefanKarpinski
Copy link
Sponsor Member

Ideally this should really be necessary since the comparison operation between characters is just an integer comparison so it seems like something else is not quite right here if this is necessary for good performance.

@KristofferC
Copy link
Sponsor Member Author

It is not the comparison that is expensive, it is the arithmetic (- in this case), see #28671.

However, hoisting the conversion like this still has a ~4% performance improvement so might as well leave it?

@KristofferC
Copy link
Sponsor Member Author

@MasonProtter

0.6:

julia> @btime parseintperf(1000);
  143.515 μs (2000 allocations: 93.75 KiB)

julia> @btime f(strings)
  793.624 μs (0 allocations: 0 bytes)

@quinnj
Copy link
Member

quinnj commented Aug 21, 2018

Parsers.jl hereby challenges you to a duel:

julia> strings = [string(rand(UInt32)) for i in 1:10^4];

julia> f(strings) = for s in strings; Parsers.parse(s, Int) end;

julia> @btime f(strings)
  549.167 μs (30000 allocations: 1.37 MiB)

@KristofferC
Copy link
Sponsor Member Author

# Base
julia> @btime f(strings)
  whatevs (0 allocations: 0 bytes)

# Parsers.jl
julia> @btime f(strings)
  whatevs (30000 allocations: 1.37 MiB)

Well?

@KristofferC KristofferC mentioned this pull request Aug 21, 2018
@KristofferC
Copy link
Sponsor Member Author

Even though this comes with a small readability regression, I think it is worth it for such a basic function (we also show parseintperf on the benchmark plot on the homepage).

@KristofferC KristofferC merged commit 81850b6 into master Aug 22, 2018
@KristofferC KristofferC deleted the kc/perf_parse2 branch August 22, 2018 10:10
KristofferC added a commit that referenced this pull request Aug 23, 2018
(cherry picked from commit 81850b6)
staticfloat pushed a commit that referenced this pull request Aug 24, 2018
KristofferC added a commit that referenced this pull request Sep 8, 2018
(cherry picked from commit 81850b6)
KristofferC added a commit that referenced this pull request Sep 8, 2018
(cherry picked from commit 81850b6)
@KristofferC
Copy link
Sponsor Member Author

@quinnj Getting closer ;)

julia> @btime f(strings)
  595.071 μs (0 allocations: 0 bytes)

KristofferC added a commit that referenced this pull request Feb 11, 2019
(cherry picked from commit 81850b6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:strings "Strings!" performance Must go faster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants