Skip to content

support skipchars and peek(io, Char)#77

Merged
KristofferC merged 4 commits intomasterfrom
skipchars
Aug 14, 2023
Merged

support skipchars and peek(io, Char)#77
KristofferC merged 4 commits intomasterfrom
skipchars

Conversation

@stevengj
Copy link
Copy Markdown
Member

@stevengj stevengj commented Jul 12, 2023

Closes #57 by implementing skipchars and peek(io, Char), and also optimizing read(io, Char), for BufferedInputStream.

@stevengj
Copy link
Copy Markdown
Member Author

stevengj commented Jul 12, 2023

Note that the peek tests in this PR fail on master, where it calls Base's generic peek(io, T) implementation based on mark and reset. Seems like that indicates a bug somewhere else? I will try to make a minimal reproducer.

Update: see #78

@codecov
Copy link
Copy Markdown

codecov bot commented Jul 12, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +1.46 🎉

Comparison is base (00bf5e8) 85.82% compared to head (763833c) 87.29%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #77      +/-   ##
==========================================
+ Coverage   85.82%   87.29%   +1.46%     
==========================================
  Files           5        5              
  Lines         381      425      +44     
==========================================
+ Hits          327      371      +44     
  Misses         54       54              
Impacted Files Coverage Δ
src/bufferedinputstream.jl 97.54% <100.00%> (+0.54%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@stevengj
Copy link
Copy Markdown
Member Author

stevengj commented Jul 12, 2023

CI failure is due to JuliaLang/julia#50532 — need to decide what we want the behavior to be.

Update:: fixed following JuliaLang/julia#50552

@stevengj
Copy link
Copy Markdown
Member Author

stevengj commented Jul 14, 2023

For the record, on a simple benchmark

s = randstring("xα∆🐨", 100) * ' '
io = BufferedInputStream(IOBuffer(s));
@btime skipchars(!=(' '), seekstart($io));

this is about 20% faster than the much simpler implementation:

function skipchars(predicate, stream::BufferedInputStream; linecomment=nothing)
    BufferedStreams.checkopen(stream)
    while !eof(stream)
        mark(stream)
        c = read(stream, Char)
        if c === linecomment
            error("unimplemented") # not benchmarking line comments
        elseif predicate(c)
            unmark(stream)
        else
            reset(stream)
            break
        end
    end
    return stream
end

which seems worth it for another 20 lines of code for _readchar(::BufferedInputStream), since fast character-by-character I/O is an important use of buffered input.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

error in skipchars(BufferedInputStream)

2 participants