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 ascii and utf8 and some ccall usage #15773

Merged
merged 2 commits into from
Apr 7, 2016
Merged

Fix ascii and utf8 and some ccall usage #15773

merged 2 commits into from
Apr 7, 2016

Conversation

yuyichao
Copy link
Contributor

@yuyichao yuyichao commented Apr 5, 2016

  1. Make sure utf8, ascii and bytestring always check if the pointer is NULL.
  2. Avoid useless copy and dynamic dispatch in ascii and utf8.
  3. Fix some out-of-date ccall usage to reduce avoid a few type assertions.

@yuyichao yuyichao added performance Must go faster potential benchmark Could make a good benchmark in BaseBenchmarks labels Apr 5, 2016
@yuyichao
Copy link
Contributor Author

yuyichao commented Apr 5, 2016

..... Does @github not support force pushing to update a pull request anymore??

@timholy
Copy link
Member

timholy commented Apr 5, 2016

I'm having trouble over in #13323, too.

@yuyichao yuyichao force-pushed the yyc/fix-utf8 branch 2 times, most recently from 2e87068 to 057442e Compare April 5, 2016 21:53
@Keno
Copy link
Member

Keno commented Apr 5, 2016

@yuyichao
Copy link
Contributor Author

yuyichao commented Apr 5, 2016

thx, looks good now =)

@yuyichao yuyichao force-pushed the yyc/fix-utf8 branch 2 times, most recently from 732a416 to 19b041a Compare April 6, 2016 01:52
yuyichao added 2 commits April 6, 2016 18:51
Make conversion from c pointer to `ByteString`, `ASCIIString` and `UTF8String`
more efficient and consistent.
yuyichao added a commit to yuyichao/explore that referenced this pull request Apr 6, 2016
@yuyichao
Copy link
Contributor Author

yuyichao commented Apr 6, 2016

Using the micro benchmarks here

using Benchmarks

function show_benchmark_result(r)
    stats = Benchmarks.SummaryStatistics(r)
    max_length = 24
    if isnull(stats.elapsed_time_lower) || isnull(stats.elapsed_time_upper)
        @printf("%s: %s\n",
                Benchmarks.lpad("Time per evaluation", max_length),
                Benchmarks.pretty_time_string(stats.elapsed_time_center))
    else
        @printf("%s: %s [%s, %s]\n",
                Benchmarks.lpad("Time per evaluation", max_length),
                Benchmarks.pretty_time_string(stats.elapsed_time_center),
                Benchmarks.pretty_time_string(get(stats.elapsed_time_lower)),
                Benchmarks.pretty_time_string(get(stats.elapsed_time_upper)))
    end
end

macro show_benchmark(ex)
    quote
        println($(QuoteNode(ex)))
        show_benchmark_result(@benchmark $(esc(ex)))
    end
end

ascii_strs = ["0123"^i for i in (1, 10, 100, 1000)]
utf8_strs = ["αβγδ"^i for i in (1, 10, 100, 1000)]

println("ASCII:")
for s in ascii_strs
    sp = pointer(s)
    print(length(s), " ")
    @show_benchmark utf8(sp)
    print(length(s), " ")
    @show_benchmark utf8(sp, 4)
    print(length(s), " ")
    @show_benchmark ascii(sp)
    print(length(s), " ")
    @show_benchmark ascii(sp, 4)
end

println("UTF8:")
for s in utf8_strs
    sp = pointer(s)
    print(length(s), " ")
    @show_benchmark utf8(sp)
    print(length(s), " ")
    @show_benchmark utf8(sp, 8)
end

Here's the difference in the performance...

--- old 2016-04-06 19:13:39.020357001 -0400
+++ new 2016-04-06 19:12:30.263410217 -0400
@@ -1,50 +1,50 @@
 ASCII:
 4 utf8(sp)
-     Time per evaluation: 161.08 ns [157.90 ns, 164.25 ns]
+     Time per evaluation: 82.06 ns [80.48 ns, 83.65 ns]
 4 utf8(sp,4)
-     Time per evaluation: 201.50 ns [197.38 ns, 205.62 ns]
+     Time per evaluation: 84.34 ns [82.71 ns, 85.96 ns]
 4 ascii(sp)
-     Time per evaluation: 142.29 ns [139.31 ns, 145.28 ns]
+     Time per evaluation: 90.84 ns [88.94 ns, 92.73 ns]
 4 ascii(sp,4)
-     Time per evaluation: 85.85 ns [84.11 ns, 87.58 ns]
+     Time per evaluation: 89.44 ns [87.61 ns, 91.27 ns]
 40 utf8(sp)
-     Time per evaluation: 197.29 ns [193.21 ns, 201.38 ns]
+     Time per evaluation: 115.88 ns [113.91 ns, 117.84 ns]
 40 utf8(sp,4)
-     Time per evaluation: 186.16 ns [182.02 ns, 190.31 ns]
+     Time per evaluation: 89.39 ns [87.50 ns, 91.27 ns]
 40 ascii(sp)
-     Time per evaluation: 172.80 ns [168.85 ns, 176.75 ns]
+     Time per evaluation: 133.23 ns [130.67 ns, 135.79 ns]
 40 ascii(sp,4)
-     Time per evaluation: 87.99 ns [86.20 ns, 89.77 ns]
+     Time per evaluation: 86.12 ns [84.42 ns, 87.81 ns]
 400 utf8(sp)
-     Time per evaluation: 470.00 ns [459.50 ns, 480.51 ns]
+     Time per evaluation: 252.85 ns [247.65 ns, 258.05 ns]
 400 utf8(sp,4)
-     Time per evaluation: 199.70 ns [195.20 ns, 204.20 ns]
+     Time per evaluation: 84.45 ns [82.80 ns, 86.10 ns]
 400 ascii(sp)
-     Time per evaluation: 462.67 ns [452.84 ns, 472.50 ns]
+     Time per evaluation: 410.02 ns [400.55 ns, 419.49 ns]
 400 ascii(sp,4)
-     Time per evaluation: 91.41 ns [89.50 ns, 93.31 ns]
+     Time per evaluation: 87.52 ns [85.93 ns, 89.11 ns]
 4000 utf8(sp)
-     Time per evaluation: 2.18 μs [2.13 μs, 2.23 μs]
+     Time per evaluation: 848.01 ns [836.11 ns, 859.91 ns]
 4000 utf8(sp,4)
-     Time per evaluation: 197.69 ns [193.64 ns, 201.74 ns]
+     Time per evaluation: 82.98 ns [81.42 ns, 84.53 ns]
 4000 ascii(sp)
-     Time per evaluation: 2.10 μs [2.05 μs, 2.15 μs]
+     Time per evaluation: 2.08 μs [2.03 μs, 2.13 μs]
 4000 ascii(sp,4)
-     Time per evaluation: 88.43 ns [86.73 ns, 90.13 ns]
+     Time per evaluation: 84.14 ns [82.55 ns, 85.73 ns]
 UTF8:
 4 utf8(sp)
-     Time per evaluation: 163.07 ns [159.61 ns, 166.53 ns]
+     Time per evaluation: 83.08 ns [81.48 ns, 84.67 ns]
 4 utf8(sp,8)
-     Time per evaluation: 199.50 ns [195.06 ns, 203.93 ns]
+     Time per evaluation: 80.24 ns [78.65 ns, 81.84 ns]
 40 utf8(sp)
-     Time per evaluation: 259.66 ns [254.55 ns, 264.78 ns]
+     Time per evaluation: 130.54 ns [128.34 ns, 132.74 ns]
 40 utf8(sp,8)
-     Time per evaluation: 193.80 ns [189.88 ns, 197.72 ns]
+     Time per evaluation: 82.75 ns [81.10 ns, 84.41 ns]
 400 utf8(sp)
-     Time per evaluation: 996.22 ns [970.59 ns, 1.02 μs]
+     Time per evaluation: 361.85 ns [354.73 ns, 368.96 ns]
 400 utf8(sp,8)
-     Time per evaluation: 201.93 ns [197.81 ns, 206.05 ns]
+     Time per evaluation: 82.45 ns [80.74 ns, 84.17 ns]
 4000 utf8(sp)
-     Time per evaluation: 6.87 μs [6.66 μs, 7.08 μs]
+     Time per evaluation: 1.17 μs [1.15 μs, 1.19 μs]
 4000 utf8(sp,8)
-     Time per evaluation: 205.73 ns [201.35 ns, 210.12 ns]
+     Time per evaluation: 77.77 ns [76.27 ns, 79.27 ns]

@yuyichao yuyichao merged commit a51c6fb into master Apr 7, 2016
@yuyichao yuyichao deleted the yyc/fix-utf8 branch April 7, 2016 13:31
yuyichao added a commit to yuyichao/LibALPM.jl that referenced this pull request Apr 11, 2016
yuyichao added a commit to yuyichao/LibALPM.jl that referenced this pull request Apr 11, 2016
@KristofferC KristofferC removed the potential benchmark Could make a good benchmark in BaseBenchmarks label Oct 31, 2018
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.

4 participants