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

add sizehint keyword argument to IOBuffer #25944

Merged
merged 8 commits into from
Feb 9, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,9 @@ Library improvements
like other `AbstractDict` subtypes and its constructors mirror the
ones of `Dict`. ([#25210])

* `IOBuffer` can take the `sizehint` keyword argument to suggest a capacity of
the buffer ([#25944]).

Compiler/Runtime improvements
-----------------------------

Expand Down
12 changes: 9 additions & 3 deletions base/iobuffer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ It may take optional keyword arguments:
- `read`, `write`, `append`: restricts operations to the buffer; see `open` for details.
- `truncate`: truncates the buffer size to zero length.
- `maxsize`: specifies a size beyond which the buffer may not be grown.
- `sizehint`: suggests a capacity of the buffer (`data` must implement `sizehint!(data, size)`).

When `data` is not given, the buffer will be both readable and writable by default.

Expand Down Expand Up @@ -84,10 +85,14 @@ function IOBuffer(
write::Union{Bool,Nothing}=nothing,
append::Union{Bool,Nothing}=nothing,
truncate::Union{Bool,Nothing}=nothing,
maxsize::Integer=typemax(Int))
maxsize::Integer=typemax(Int),
sizehint::Union{Integer,Nothing}=nothing)
if maxsize < 0
throw(ArgumentError("negative maxsize: $(maxsize)"))
end
if sizehint !== nothing
sizehint!(data, sizehint)
end
flags = open_flags(read=read, write=write, append=append, truncate=truncate)
buf = GenericIOBuffer(data, flags.read, flags.write, true, flags.append, Int(maxsize))
if flags.truncate
Expand All @@ -101,8 +106,9 @@ function IOBuffer(;
write::Union{Bool,Nothing}=true,
append::Union{Bool,Nothing}=nothing,
truncate::Union{Bool,Nothing}=true,
maxsize::Integer=typemax(Int))
size = maxsize == typemax(Int) ? 32 : Int(maxsize)
maxsize::Integer=typemax(Int),
sizehint::Union{Integer,Nothing}=nothing)
size = maxsize != typemax(Int) ? Int(maxsize) : sizehint !== nothing ? Int(sizehint) : 32
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like sizehint should take precedence over maxsize here.

Copy link
Member

@stevengj stevengj Feb 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

e.g. consider a case where maxsize is 2GB (because the buffer will be sent someplace 32-bit) but sizehint is 8. We probably don't want to allocate 2GB.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot to fix that. Now fixed. Thank you.

flags = open_flags(read=read, write=write, append=append, truncate=truncate)
buf = IOBuffer(
StringVector(size),
Expand Down
4 changes: 2 additions & 2 deletions base/strings/basic.jl
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ isascii(s::AbstractString) = all(isascii, s)
## string map, filter, has ##

function map(f, s::AbstractString)
out = IOBuffer(StringVector(sizeof(s)), read=true, write=true)
out = IOBuffer(sizehint=sizeof(s), read=true, write=true)
truncate(out, 0)
for c in s
c′ = f(c)
Expand All @@ -525,7 +525,7 @@ function map(f, s::AbstractString)
end

function filter(f, s::AbstractString)
out = IOBuffer(StringVector(sizeof(s)), read=true, write=true)
out = IOBuffer(sizehint=sizeof(s), read=true, write=true)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We no longer have to pass read=true, write=true, since this is the default when no data buffer is passed to the constructor. (Similarly elsewhere.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i.e. just call IOBuffer(sizehint=sizeof(s)).

truncate(out, 0)
for c in s
f(c) && write(out, c)
Expand Down
6 changes: 3 additions & 3 deletions base/strings/io.jl
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ julia> sprint(showcompact, 66.66666)
```
"""
function sprint(f::Function, args...; context=nothing, sizehint::Integer=0)
s = IOBuffer(StringVector(sizehint), read=true, write=true)
s = IOBuffer(sizehint=sizehint, read=true, write=true)
# specialized version of truncate(s,0)
s.size = 0
s.ptr = 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't the these three lines be deleted?

Expand All @@ -100,7 +100,7 @@ tostr_sizehint(x::Float32) = 12

function print_to_string(xs...; env=nothing)
# specialized for performance reasons
s = IOBuffer(StringVector(tostr_sizehint(xs[1])), read=true, write=true)
s = IOBuffer(sizehint=tostr_sizehint(xs[1]), read=true, write=true)
Copy link
Member

@stevengj stevengj Feb 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't there be an isempty(xs) && return "" check? Or is this an internal function that can never be called on an empty argument list, in which case there should be an @assert !isempty(xs)?

# specialized version of truncate(s,0)
s.size = 0
s.ptr = 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't the these three lines be deleted?

Expand Down Expand Up @@ -436,7 +436,7 @@ Returns:
function unindent(str::AbstractString, indent::Int; tabwidth=8)
indent == 0 && return str
# Note: this loses the type of the original string
buf = IOBuffer(StringVector(sizeof(str)), read=true, write=true)
buf = IOBuffer(sizehint=sizeof(str), read=true, write=true)
truncate(buf,0)
cutting = true
col = 0 # current column (0 based)
Expand Down
2 changes: 1 addition & 1 deletion base/strings/util.jl
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ function replace(str::String, pat_repl::Pair; count::Integer=typemax(Int))
i = a = firstindex(str)
r = coalesce(findnext(pattern,str,i), 0)
j, k = first(r), last(r)
out = IOBuffer(StringVector(floor(Int, 1.2sizeof(str))), read=true, write=true)
out = IOBuffer(sizehint=floor(Int, 1.2sizeof(str)), read=true, write=true)
out.size = 0
out.ptr = 1
while j != 0
Expand Down