Skip to content

Commit

Permalink
Merge pull request #21117 from fcard/fix-skipchars-regression-interme…
Browse files Browse the repository at this point in the history
…diary

Improve performance of skipchars for small inputs
  • Loading branch information
JeffBezanson committed Mar 21, 2017
2 parents 91f3275 + 2894595 commit 90cfb82
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 8 deletions.
2 changes: 1 addition & 1 deletion base/iostream.jl
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ function skipchars(io::IOStream, pred; linecomment=nothing)
if c === linecomment
readline(io)
elseif !pred(c)
seek(io,position(io)-sizeof(string(c)))
skip(io, -codelen(c))
break
end
end
Expand Down
22 changes: 15 additions & 7 deletions test/iostream.jl
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@
mktemp() do path, file
function append_to_file(str)
mark(file)
println(file, str)
print(file, str)
flush(file)
reset(file)
end

# test it doesn't error on eof
@test skipchars(file, isspace) == file
@test eof(skipchars(file, isspace))

# test if it correctly skips
# test it correctly skips
append_to_file(" ")
@test eof(skipchars(file, isspace))

Expand All @@ -22,12 +22,20 @@ mktemp() do path, file

# test it stops at the appropriate time
append_to_file(" not a space")
@test skipchars(file, isspace) == file
@test !eof(file) && read(file, Char) == 'n'
@test !eof(skipchars(file, isspace))
@test read(file, Char) == 'n'

# test it correctly ignores the contents of comment lines
append_to_file(" #not a space \n not a space")
@test skipchars(file, isspace, linecomment='#') == file
@test !eof(file) && read(file, Char) == 'n'
@test !eof(skipchars(file, isspace, linecomment='#'))
@test read(file, Char) == 'n'

# test it correctly handles unicode
for (byte,char) in zip(1:4, ('@','߷','','𐋺'))
append_to_file("abcdef$char")
@test Base.codelen(char) == byte
@test !eof(skipchars(file, isalpha))
@test read(file, Char) == char
end
end

5 comments on commit 90cfb82

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels

@martinholters
Copy link
Member

Choose a reason for hiding this comment

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

The "array","comprehension" ones look real?

@StefanKarpinski
Copy link
Member

Choose a reason for hiding this comment

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

That seems impossible to be related to this.

@nalimilan
Copy link
Member

Choose a reason for hiding this comment

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

This is the daily benchmark, though, so it could be due to another commit.

Please sign in to comment.