From a84c71dbad212b07f9367edd32d011ffdac3dd26 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Sun, 7 Oct 2018 15:28:01 -0300 Subject: [PATCH] Let Zlib::Reader, Gzip::Reader and Flate::Reader include IO::Buffered for optimized performance --- spec/std/flate/flate_spec.cr | 23 ++++++++--- spec/std/gzip/gzip_spec.cr | 76 +++++++++++++++++++++--------------- spec/std/zlib/reader_spec.cr | 29 +++++++++----- src/flate/reader.cr | 24 ++++++++++-- src/gzip/reader.cr | 23 +++++++++-- src/zlib/reader.cr | 20 ++++++++-- 6 files changed, 140 insertions(+), 55 deletions(-) diff --git a/spec/std/flate/flate_spec.cr b/spec/std/flate/flate_spec.cr index c5188075e796..f2071b571d76 100644 --- a/spec/std/flate/flate_spec.cr +++ b/spec/std/flate/flate_spec.cr @@ -1,14 +1,18 @@ require "spec" require "flate" +private def new_sample_io + io = IO::Memory.new + "cbc9cc4b350402ae1c20c30808b800".scan(/../).each do |match| + io.write_byte match[0].to_u8(16) + end + io.rewind +end + module Flate describe Reader do it "should read byte by byte (#4192)" do - io = IO::Memory.new - "cbc9cc4b350402ae1c20c30808b800".scan(/../).each do |match| - io.write_byte match[0].to_u8(16) - end - io.rewind + io = new_sample_io reader = Reader.new(io) @@ -21,6 +25,15 @@ module Flate str.should eq("line1111\nline2222\n") end + it "should rewind" do + io = new_sample_io + + reader = Reader.new(io) + reader.gets.should eq("line1111") + reader.rewind + reader.gets_to_end.should eq("line1111\nline2222\n") + end + describe ".open" do it "yields itself to block" do # Hello Crystal! diff --git a/spec/std/gzip/gzip_spec.cr b/spec/std/gzip/gzip_spec.cr index 2cdd9ecc2bb1..249855d8c899 100644 --- a/spec/std/gzip/gzip_spec.cr +++ b/spec/std/gzip/gzip_spec.cr @@ -1,46 +1,60 @@ require "spec" require "gzip" +private SAMPLE_TIME = Time.utc(2016, 1, 2) +private SAMPLE_OS = 4_u8 +private SAMPLE_EXTRA = Bytes[1, 2, 3] +private SAMPLE_NAME = "foo.txt" +private SAMPLE_COMMENT = "some comment" +private SAMPLE_CONTENTS = "hello world\nfoo bar" + +private def new_sample_io + io = IO::Memory.new + + Gzip::Writer.open(io) do |gzip| + header = gzip.header + header.modification_time = SAMPLE_TIME + header.os = SAMPLE_OS + header.extra = SAMPLE_EXTRA + header.name = SAMPLE_NAME + header.comment = SAMPLE_COMMENT + + io.bytesize.should eq(0) + gzip.flush + io.bytesize.should_not eq(0) + + gzip.print SAMPLE_CONTENTS + end + + io.rewind +end + describe Gzip do it "writes and reads to memory" do - io = IO::Memory.new - - time = Time.utc(2016, 1, 2) - os = 4_u8 - extra = Bytes[1, 2, 3] - name = "foo.txt" - comment = "some comment" - contents = "hello world" - - Gzip::Writer.open(io) do |gzip| - header = gzip.header - header.modification_time = time - header.os = os - header.extra = extra - header.name = name - header.comment = comment - - io.bytesize.should eq(0) - gzip.flush - io.bytesize.should_not eq(0) - - gzip.print contents - end - - io.rewind + io = new_sample_io Gzip::Reader.open(io) do |gzip| header = gzip.header.not_nil! - header.modification_time.should eq(time) - header.os.should eq(os) - header.extra.should eq(extra) - header.name.should eq(name) - header.comment.should eq(comment) + header.modification_time.should eq(SAMPLE_TIME) + header.os.should eq(SAMPLE_OS) + header.extra.should eq(SAMPLE_EXTRA) + header.name.should eq(SAMPLE_NAME) + header.comment.should eq(SAMPLE_COMMENT) # Reading zero bytes is OK gzip.read(Bytes.empty).should eq(0) - gzip.gets_to_end.should eq(contents) + gzip.gets_to_end.should eq(SAMPLE_CONTENTS) end end + + it "rewinds" do + io = new_sample_io + + gzip = Gzip::Reader.new(io) + gzip.gets.should eq(SAMPLE_CONTENTS.lines.first) + + gzip.rewind + gzip.gets_to_end.should eq(SAMPLE_CONTENTS) + end end diff --git a/spec/std/zlib/reader_spec.cr b/spec/std/zlib/reader_spec.cr index b098d6d3cbb5..0d48b8eb2729 100644 --- a/spec/std/zlib/reader_spec.cr +++ b/spec/std/zlib/reader_spec.cr @@ -1,14 +1,18 @@ require "spec" require "zlib" +private def new_sample_io + io = IO::Memory.new + "789c2bc9c82c5600a2448592d4e21285e292a2ccbc74054520e00200854f087b".scan(/../).each do |match| + io.write_byte match[0].to_u8(16) + end + io.rewind +end + module Zlib describe Reader do it "should be able to read" do - io = IO::Memory.new - "789c2bc9c82c5600a2448592d4e21285e292a2ccbc74054520e00200854f087b".scan(/../).each do |match| - io.write_byte match[0].to_u8(16) - end - io.rewind + io = new_sample_io reader = Reader.new(io) @@ -20,6 +24,15 @@ module Zlib reader.read(Bytes.new(10)).should eq(0) end + it "rewinds" do + io = new_sample_io + + reader = Reader.new(io) + reader.gets(3).should eq("thi") + reader.rewind + reader.gets_to_end.should eq("this is a test string !!!!\n") + end + it "can be closed without sync" do io = IO::Memory.new(Bytes[120, 156, 3, 0, 0, 0, 0, 1]) reader = Reader.new(io) @@ -56,11 +69,7 @@ module Zlib end it "should not freeze when reading empty slice" do - io = IO::Memory.new - "789c2bc9c82c5600a2448592d4e21285e292a2ccbc74054520e00200854f087b".scan(/../).each do |match| - io.write_byte match[0].to_u8(16) - end - io.rewind + io = new_sample_io reader = Reader.new(io) slice = Bytes.empty reader.read(slice).should eq(0) diff --git a/src/flate/reader.cr b/src/flate/reader.cr index f66e73984281..24a9427e87b0 100644 --- a/src/flate/reader.cr +++ b/src/flate/reader.cr @@ -4,12 +4,17 @@ # instance, it reads data from the underlying IO, decompresses it, and returns # it to the caller. class Flate::Reader < IO + include IO::Buffered + # If `#sync_close?` is `true`, closing this IO will close the underlying IO. property? sync_close : Bool # Returns `true` if this reader is closed. getter? closed = false + # Dictionary passed in the constructor + getter dict : Bytes? + # Peeked bytes from the underlying IO @peek : Bytes? @@ -22,6 +27,7 @@ class Flate::Reader < IO ret = LibZ.inflateInit2(pointerof(@stream), -LibZ::MAX_BITS, LibZ.zlibVersion, sizeof(LibZ::ZStream)) raise Flate::Error.new(ret, @stream) unless ret.ok? + @peek = nil @end = false end @@ -46,12 +52,12 @@ class Flate::Reader < IO end # Always raises `IO::Error` because this is a read-only `IO`. - def write(slice : Bytes) + def unbuffered_write(slice : Bytes) raise IO::Error.new "Can't write to Flate::Reader" end # See `IO#read`. - def read(slice : Bytes) + def unbuffered_read(slice : Bytes) check_open return 0 if slice.empty? @@ -119,8 +125,12 @@ class Flate::Reader < IO end end + def unbuffered_flush + raise IO::Error.new "Can't flush Flate::Reader" + end + # Closes this reader. - def close + def unbuffered_close return if @closed @closed = true @@ -130,6 +140,14 @@ class Flate::Reader < IO @io.close if @sync_close end + def unbuffered_rewind + raise IO::Error.new "Closed stream" if closed? + + @io.rewind + + initialize(@io, @sync_close, @dict) + end + # :nodoc: def inspect(io) to_s(io) diff --git a/src/gzip/reader.cr b/src/gzip/reader.cr index fbbf6fcc6803..9f7cef0ae903 100644 --- a/src/gzip/reader.cr +++ b/src/gzip/reader.cr @@ -27,6 +27,8 @@ # string # => "abc" # ``` class Gzip::Reader < IO + include IO::Buffered + # Whether to close the enclosed `IO` when closing this reader. property? sync_close = false @@ -73,7 +75,7 @@ class Gzip::Reader < IO end # See `IO#read`. - def read(slice : Bytes) + def unbuffered_read(slice : Bytes) check_open return 0 if slice.empty? @@ -121,16 +123,31 @@ class Gzip::Reader < IO end # Always raises `IO::Error` because this is a read-only `IO`. - def write(slice : Bytes) : Nil + def unbuffered_write(slice : Bytes) : Nil raise IO::Error.new("Can't write to Gzip::Reader") end + def unbuffered_flush + raise IO::Error.new "Can't flush Gzip::Reader" + end + # Closes this reader. - def close + def unbuffered_close return if @closed @closed = true @flate_io.try &.close @io.close if @sync_close end + + def unbuffered_rewind + raise IO::Error.new "Closed stream" if closed? + + @io.rewind + + @header = nil + @flate_io = nil + + initialize(@io, @sync_close) + end end diff --git a/src/zlib/reader.cr b/src/zlib/reader.cr index 846058ab33e4..b546e2127882 100644 --- a/src/zlib/reader.cr +++ b/src/zlib/reader.cr @@ -4,6 +4,8 @@ # instance, it reads data from the underlying IO, decompresses it, and returns # it to the caller. class Zlib::Reader < IO + include IO::Buffered + # Whether to close the enclosed `IO` when closing this reader. property? sync_close = false @@ -56,7 +58,7 @@ class Zlib::Reader < IO end # See `IO#read`. - def read(slice : Bytes) + def unbuffered_read(slice : Bytes) check_open return 0 if slice.empty? @@ -79,11 +81,15 @@ class Zlib::Reader < IO end # Always raises `IO::Error` because this is a read-only `IO`. - def write(slice : Bytes) + def unbuffered_write(slice : Bytes) raise IO::Error.new "Can't write to Zlib::Reader" end - def close + def unbuffered_flush + raise IO::Error.new "Can't flush Zlib::Reader" + end + + def unbuffered_close return if @closed @closed = true @@ -91,6 +97,14 @@ class Zlib::Reader < IO @io.close if @sync_close end + def unbuffered_rewind + raise IO::Error.new "Closed stream" if closed? + + @io.rewind + + initialize(@io, @sync_close, @flate_io.dict) + end + protected def self.invalid_header raise Zlib::Error.new("Invalid header") end