Improve countlines() performance

[quinnj]

Size of File	Base.countlines	new countlines
10MB	11.3 milliseconds	7.3 milliseconds
100MB	102 milliseconds	62.5 milliseconds
1GB	928 milliseconds	598 milliseconds
21GB	31 seconds	26 seconds

The first 3 files were auto-generated from random characters + newlines. The 21GB file is from @jiahao's test file from the readdlm issue.

Also added previously missing tests

[jiahao]

I restarted two builds - one had a weird type inference error and the other had a network failure.

[quinnj]

Yeah, the failures seemed unrealted. Let's wait on AppVeyor then merge. This isn't anything spectacular, but close to 2x improvements in some cases is nice.

[jiahao]

Glad to see improvements on the data ingest end of the standard library!

[carnaval]

Is it computing the same thing ? The old impl. seems to not be counting empty lines.

[quinnj]

Yeah, I noticed that. I'm not sure why we wouldn't count empty lines? Looking at the source code for wc it didn't have anything like that.

[quinnj]

We could maybe provide a skip_emtpy argument, but I'd really like to know if anyone had a use for this.

[ScottPJones]

👍 LGTM

[mbauman]

Well, it was introduced in 2eb2493 with a reference to dlmread. I didn't dig any further, but is it (or was it) used to determine the size of an array to preallocate?

[ScottPJones]

OK, I've looked everywhere. This is documented as counting non-empty lines, however, it is not used anywhere in Base, and is only used in one place in any registered package, in Jumos/src/Trajectories/XYZ/Reader.jl, where it is just verifying that the number of lines stored in the header of the file matches the rest of the lines in the file, not performance critical.

[quinnj]

Thanks for looking around @ScottPJones. My guess is it used to be used by readdlm, but now it's using something else. Sounds like the breaking nature here won't affect much, so I'm inclined to merge and we can add another argument later if a compelling use case comes up. I'll update the docs.

[ScottPJones]

Yes, I'm sad though, because your code looks very nice and fast, only to find out that it's not (currently) used except that once in a package.
Better to merge it in, have the faster,better code for anybody in the future who needs it!

[quinnj]

Ref: Luthaf/Jumos.jl#22

[quinnj]

Interesting tidbit for others out there: while the open(func, file) can be quick and tidy, it's also quite subtle from a type stability perspective. Without the ::Int assertion here, @code_warntype shows that the return type for this method is Any, even though countlines itself is type-stable.