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

support skipchars and peek(io, Char) #77

Merged
merged 4 commits into from
Aug 14, 2023
Merged

support skipchars and peek(io, Char) #77

merged 4 commits into from
Aug 14, 2023

Conversation

stevengj
Copy link
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
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

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
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
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.

@KristofferC KristofferC merged commit 9602735 into master Aug 14, 2023
7 checks passed
@KristofferC KristofferC deleted the skipchars branch August 14, 2023 11:46
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