ancapdev / LightBSON.jl

High performance encoding and decoding of BSON data in Julia
MIT License
20 stars 4 forks source link

reducible with BSONIndexUnsafe #26

Open poncito opened 1 year ago

poncito commented 1 year ago

Implements a more efficient "break" when foldl-ing. I just reduce on an isbit type, that I call "BSONIndexUnsafe", instead of a BSONReader.

I test:

julia> buf = UInt8[]
       writer = BSONWriter(buf)
       writer[] = (;a=[1,2,3])
       close(writer)

       @btime begin
           reader = BSONReader($buf)
           reader["a"]["2"][Int64]
       end

       @btime begin
           reader = BSONReader($buf)
           reader["a"][3][Int64]
       end

Before:

17.576 ns (0 allocations: 0 bytes)
108.146 ns (2 allocations: 64 bytes)

and after

 17.618 ns (0 allocations: 0 bytes)
 17.911 ns (0 allocations: 0 bytes)

A few remarks:

  1. I'd like to remove the code duplication between:
    • function Transducers.__foldl__(rf, val, reader::BSONReader)
    • function Transducers.__foldl__(rf, val, indices::BSONIndices) but I don't know how, but it should be doable, no? Obviously the first method would rely on the second one.
  2. I'd like to remove the code duplication between:
    • Base.getindex(reader::BSONReader, target::Union{AbstractString, Symbol})
    • function Transducers.__foldl__(rf, val, indices::BSONIndices) That seems more tricky because of the name_len_and_match_ optimization.
  3. You will notice that the field el_type of BSONIndexUnsafe is of type Int. It should be a UInt8. But if I do that, the benchmark above runs in 60ns. I don't understand why my "fix" as any kind of impact. Any idea?

Thanks,

codecov-commenter commented 1 year ago

Codecov Report

Merging #26 (7998042) into master (ed7432c) will decrease coverage by 2.63%. The diff coverage is 94.73%.

@@            Coverage Diff             @@
##           master      #26      +/-   ##
==========================================
- Coverage   92.35%   89.71%   -2.64%     
==========================================
  Files          13       13              
  Lines         824      846      +22     
==========================================
- Hits          761      759       -2     
- Misses         63       87      +24     
Flag Coverage Δ
unittest 89.71% <94.73%> (-2.64%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/reader.jl 92.05% <94.73%> (-1.28%) :arrow_down:
src/writer.jl 88.82% <0.00%> (-8.46%) :arrow_down:
src/object_id.jl 86.04% <0.00%> (-6.98%) :arrow_down:
src/LightBSON.jl 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

ancapdev commented 1 year ago

@Poncito just letting you know I've seen the PR, but I'm quite busy at the moment and I think I need a little time to sit down and wrap my head around the changes here. Performance results look good though :+1: