Skip to content

Commit

Permalink
Always read unbuffered when IO::Buffered#sync = true (#5849)
Browse files Browse the repository at this point in the history
* Always read unbuffered when IO::Buffered#sync = true

Previously the sync option only affected writes, which wasn't optimal.

* fixup! Always read unbuffered when IO::Buffered#sync = true

* fixup! Always read unbuffered when IO::Buffered#sync = true
  • Loading branch information
RX14 authored May 22, 2018
1 parent 8c24616 commit 4e0dc9e
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 12 deletions.
100 changes: 91 additions & 9 deletions spec/std/io/buffered_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -284,19 +284,101 @@ describe "IO::Buffered" do
str.to_s.should eq("abcd")
end

it "syncs" do
str = IO::Memory.new
describe "sync" do
it "syncs (write)" do
str = IO::Memory.new

io = BufferedWrapper.new(str)
io.sync?.should be_false
io = BufferedWrapper.new(str)
io.sync?.should be_false

io.sync = true
io.sync?.should be_true

io.write_byte 1_u8

str.rewind
str.read_byte.should eq(1_u8)
end

it "works with IO#read" do
str = IO::Memory.new "abc"

io = BufferedWrapper.new(str)
io.sync?.should be_false

io.sync = true
io.sync?.should be_true

byte = Bytes.new(1)
io.read_fully(byte)
byte[0].should eq('a'.ord.to_u8)

str.gets_to_end.should eq("bc")
end

it "works with IO#read (already buffered)" do
str = IO::Memory.new
str << "a" * IO::Buffered::BUFFER_SIZE
str.pos = 0

io = BufferedWrapper.new(str)
io.sync?.should be_false

IO::Buffered::BUFFER_SIZE.times do
byte = Bytes.new(1)
io.read_fully(byte)
byte[0].should eq('a'.ord.to_u8)
end

io.sync = true
io.sync?.should be_true

io.sync = true
io.sync?.should be_true
str << "bcde"
str.pos -= 4

io.write_byte 1_u8
byte = Bytes.new(1)
io.read_fully(byte)
byte[0].should eq('b'.ord.to_u8)

str.rewind
str.read_byte.should eq(1_u8)
str.gets_to_end.should eq("cde")
end

it "works with IO#read_byte" do
str = IO::Memory.new "abc"

io = BufferedWrapper.new(str)
io.sync?.should be_false

io.sync = true
io.sync?.should be_true

io.read_byte.should eq('a'.ord.to_u8)

str.gets_to_end.should eq("bc")
end

it "works with IO#read_byte (already buffered)" do
str = IO::Memory.new
str << "a" * IO::Buffered::BUFFER_SIZE
str.pos = 0

io = BufferedWrapper.new(str)
io.sync?.should be_false

IO::Buffered::BUFFER_SIZE.times do
io.read_byte.should eq('a'.ord.to_u8)
end

io.sync = true
io.sync?.should be_true

str << "bcde"
str.pos -= 4

io.read_byte.should eq('b'.ord.to_u8)

str.gets_to_end.should eq("cde")
end
end

it "shouldn't call unbuffered read if reading to an empty slice" do
Expand Down
13 changes: 10 additions & 3 deletions src/io/buffered.cr
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,16 @@ module IO::Buffered
def read_byte : UInt8?
check_open

fill_buffer if @in_buffer_rem.empty?
fill_buffer if !sync? && @in_buffer_rem.empty?
if @in_buffer_rem.empty?
nil
return nil unless sync?

byte = uninitialized UInt8
if read(Slice.new(pointerof(byte), 1)) == 1
byte
else
nil
end
else
b = @in_buffer_rem[0]
@in_buffer_rem += 1
Expand All @@ -54,7 +61,7 @@ module IO::Buffered
# If we are asked to read more than half the buffer's size,
# read directly into the slice, as it's not worth the extra
# memory copy.
if count >= BUFFER_SIZE / 2
if sync? || count >= BUFFER_SIZE / 2
return unbuffered_read(slice[0, count]).to_i
else
fill_buffer
Expand Down

0 comments on commit 4e0dc9e

Please sign in to comment.