Use Vector{Vector{UInt8}} instead of Vector{AbstractVector{UInt8}} in…

[fredrikekre]

Use Vector{Vector{UInt8}} instead of Vector{AbstractVector{UInt8}} in BufferStream.

This cuts down the benchmark in JuliaWeb/HTTP.jl#662 from

julia> @btime HTTP.get(http_jl_proxy3, response_stream=devnull);
  166.789 ms (38578 allocations: 121.59 MiB)

to

julia> @btime HTTP.get(http_jl_proxy3, response_stream=devnull);
  147.080 ms (39363 allocations: 121.63 MiB)

[codecov]

Codecov Report

Merging #3 (ea002cb) into master (c296993) will increase coverage by 1.43%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master       #3      +/-   ##
==========================================
+ Coverage   94.11%   95.55%   +1.43%     
==========================================
  Files           2        2              
  Lines          85       90       +5     
==========================================
+ Hits           80       86       +6     
+ Misses          5        4       -1     

Impacted Files	Coverage Δ
src/BufferStream.jl	95.50% <100.00%> (+1.45%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c296993...ea002cb. Read the comment docs.

[fredrikekre]

Trying to measure on the proxy process instead:

 ──────────────────────────────────────────────────────────────────
                           Time                   Allocations      
                   ──────────────────────   ───────────────────────
 Tot / % measured:      42.8s / 78.6%           33.4GiB / 100%     

 Section   ncalls     time   %tot     avg     alloc   %tot      avg
 ──────────────────────────────────────────────────────────────────
 proxy1        31    11.2s  33.4%   362ms   7.59GiB  22.7%   251MiB
 proxy2        43    10.8s  31.9%   250ms   10.3GiB  30.8%   245MiB
 proxy3        65    11.7s  34.7%   180ms   15.5GiB  46.5%   244MiB
 ──────────────────────────────────────────────────────────────────

to

 ──────────────────────────────────────────────────────────────────
                          Time                   Allocations      
                  ──────────────────────   ───────────────────────
Tot / % measured:      51.5s / 69.5%           35.8GiB / 100%     

Section   ncalls     time   %tot     avg     alloc   %tot      avg
──────────────────────────────────────────────────────────────────
proxy1        32    11.6s  32.3%   361ms   7.83GiB  21.9%   251MiB
proxy2        50    12.6s  35.1%   251ms   12.0GiB  33.4%   245MiB
proxy3        67    11.7s  32.7%   175ms   16.0GiB  44.7%   244MiB
──────────────────────────────────────────────────────────────────

but measurements seem very noisy...

[fredrikekre]

The second commit takes

julia> function perf()
           buf = BufferStream()
           @sync begin
               @async begin
                   for i in 1:1024
                       write(buf, rand(UInt8, 1024))
                   end
                   close(buf)
               end
               @async begin
                   write(devnull, buf)
               end
           end
       end
perf (generic function with 1 method)

from

julia> @btime perf();
  835.635 μs (10282 allocations: 2.53 MiB)

to

julia> @btime perf();
  819.363 μs (8235 allocations: 2.50 MiB)

[fredrikekre]

Hmm, it seems like this undoes #1 but I can not see where anything but Vector is used?

[staticfloat]

What do you mean by it un-does it?

[fredrikekre]

The AbstractVector -> Vector change.

[fredrikekre]

But I guess that wasn't needed since tests still pass here.

[staticfloat]

I'm not convinced that your benchmarks actually show a speedup?

[fredrikekre]

The speedup, although small (2%), is consistent between runs atleast, and also 20% reduction of allocations. It just seems nice to be type-stable when it is such a small change.

[fredrikekre]

The AbstractVector -> Vector change would "break" manual calls to append_chunk: https://github.com/staticfloat/SimpleBufferStream.jl/blob/e592419f6f0aee954a02bcde6ecf2b468d3c016c/src/BufferStream.jl#L50 since it would convert to Vector then.

[staticfloat]

The speedup, although small (2%), is consistent between runs atleast, and also 20% reduction of allocations. It just seems nice to be type-stable when it is such a small change.

Am I reading your table of benchmarks backwards? It looks to me like allocations go up from 15.5GB to 16.0GB.

[fredrikekre]

Oh, you are talking about #3 (comment)
Well, you can't look at the total there because there are different number of calls (65 vs 67). If you see the avg column it is identical.

I was talking about these benchmarks: #3 (comment)

[staticfloat]

Right, the second commit makes more sense to me than the first. I'm specifically wondering if the first commit actually helps at all.