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

fix performance in printf #28670

Merged
merged 1 commit into from
Aug 16, 2018
Merged

fix performance in printf #28670

merged 1 commit into from
Aug 16, 2018

Conversation

KristofferC
Copy link
Sponsor Member

Same story as in #28661.

Improves the benchmark in print_to_file (that we show on the benchmark plot on the homepage).

julia> using Printf

julia> function printfd(n)
           open("/dev/null", "w") do io
               for i = 1:n
                   @printf(io, "%d %d\n", i, i + 1)
               end
           end
       end

Before

julia> @btime printfd(1000)
  117.889 μs (5 allocations: 400 bytes)

After

julia> @btime printfd(1000)
  74.283 μs (5 allocations: 400 bytes)

base/printf.jl Outdated
@@ -862,8 +862,8 @@ function decode_oct(d::Integer)
digits = DIGITSs[Threads.threadid()]
@handle_zero x digits
pt = i = div((sizeof(x)<<3)-leading_zeros(x)+2,3)
while i > 0
digits[i] = '0'+(x&0x7)
@inbounds while i > 0
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

I don't think this is always in bounds --- digits is a fixed size and it seems to be possible to ask for enough digits to overflow it.

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

Ok let's remove the inbounds.

@KristofferC KristofferC merged commit 4629a21 into master Aug 16, 2018
@KristofferC KristofferC deleted the kc/fix_perf_printf branch August 16, 2018 08:25
KristofferC added a commit that referenced this pull request Aug 19, 2018
(cherry picked from commit 4629a21)
KristofferC added a commit that referenced this pull request Aug 19, 2018
(cherry picked from commit 4629a21)
@KristofferC KristofferC mentioned this pull request Aug 19, 2018
KristofferC added a commit that referenced this pull request Aug 19, 2018
(cherry picked from commit 4629a21)
KristofferC added a commit that referenced this pull request Sep 8, 2018
(cherry picked from commit 4629a21)
KristofferC added a commit that referenced this pull request Sep 8, 2018
(cherry picked from commit 4629a21)
@PallHaraldsson
Copy link
Contributor

Good to get the speed-up, while Int('0')+(x&0x7) seems to do the same, without less self-documenting 48 [ASCII code].

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

Successfully merging this pull request may close these issues.

3 participants