DataStructures.jl icon indicating copy to clipboard operation
DataStructures.jl copied to clipboard

CircularBuffer: Add benchmarks and improve performance

Open vyu opened this issue 4 years ago • 2 comments

This PR adds a set of benchmarks for CircularBuffer and also improves its performance.

Here are some PkgBenchmark reports using the benchmarks and comparing the new performance:

The benchmark file can be called directly by PkgBenchmark using the script keyword. For example, PkgBenchmark.benchmarkpkg("DataStructures"; script="benchmark/bench_circular_buffer.jl").

I've added review comments to explain the rationale behind some of the changes.

There was some inconsistency in the code over whether to use accessor functions (capacity(cb)) or dot notation (cb.capacity) to read field values. For consistency within the file, I've modified them to use accessor functions. They generate identical code since the functions get inlined.

I haven't worked on append! and fill! yet because improving those is more complicated. I might get to them in the future if this PR goes through.

Thanks!

vyu avatar Jul 08 '20 02:07 vyu

Thanks! I've modified lines with range iteration to follow your suggestion.

vyu avatar Jul 08 '20 11:07 vyu

I just noticed that setindex!(cb::CircularBuffer, item, i) returns cb. This has been the case since the beginning (when CircularBuffer was added in v0.4.3) and this PR retains that behavior. But per setindex! in Julia Base, it should really return item, not cb. I think this is a bug that ought to be fixed, but fixing it is potentially breaking if there is code out there that depends on this unconventional behavior. I can open a separate PR for it.

vyu avatar Jul 10 '20 02:07 vyu