-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
uu-tail performance drop when reading from stdin #3842
Comments
Thanks! These are excellent benchmarks. After looking into it, I'm not surprised it's slow and I'm curious how GNU does it so quickly. First, a (big) difference between files and stdin is to be expected, because for a file, we can just seek to the end of the file and find the data to display from there. For stdin, we cannot seek so we have to read through the data while remembering the last coreutils/src/uu/tail/src/tail.rs Line 1464 in 88261f3
With the ringbuffer defined here: https://github.com/uutils/coreutils/blob/main/src/uucore/src/lib/features/ringbuffer.rs This is of course not particularly fast. One thing we might try is to first scan in bigger blocks than lines to get a buffer that has at least |
@tertsdiepraam glad I can help out :) |
This fixes a bug where calling `tail - < file.txt` would result in invoking `unbounded_tail()`. However, it is a stdin redirect to a seekable regular file and therefore `bounded_tail` should be invoked as if `tail file.txt` had been called.
HI, thanks for doing the benchmarks and reporting this issue. What @tertsdiepraam wrote is of course true, however in this case you uncovered a bug in our tail implementation. The problem is not how we handle tailing stdin but with the stdin redirect from the shell. With the fix, the benchmarks look more resonable: Summary
'tail -n -10000 - < 505MB.txt' ran
1.36 ± 0.25 times faster than 'target/release/tail -n -10000 - < 505MB.txt'
5.16 ± 0.93 times faster than 'tail -n -100000 - < 505MB.txt'
5.98 ± 1.19 times faster than 'target/release/tail -n -100000 - < 505MB.txt'
32.12 ± 5.87 times faster than 'tail -n -1000000 - < 505MB.txt'
37.17 ± 7.74 times faster than 'target/release/tail -n -1000000 - < 505MB.txt' |
This fixes a bug where calling `tail - < file.txt` would result in invoking `unbounded_tail()`. However, it is a stdin redirect to a seekable regular file and therefore `bounded_tail` should be invoked as if `tail file.txt` had been called.
Excellent! If I understand correctly, this means that we're still slow when actually reading from stdin, right? For example, in a case like this:
As a side note, do you know why we're still slower in your benchmark even though we're faster in @Joining7943's first benchmark? It seems to me that there theoretically shouldn't be a difference between those two, since it's both just reading from a file. |
Yes: Summary
'cat 505MB.txt | tail -n -10000' ran
1.25 ± 0.07 times faster than 'cat 505MB.txt | tail -n -100000'
1.58 ± 0.07 times faster than 'cat 505MB.txt | tail -n -1000000'
1.80 ± 0.08 times faster than 'cat 505MB.txt | target/release/tail -n -10000'
2.11 ± 0.09 times faster than 'cat 505MB.txt | target/release/tail -n -100000'
4.52 ± 0.17 times faster than 'cat 505MB.txt | target/release/tail -n -1000000'
I don't know, but I agree, there shouldn't be a difference. Summary
'tail -n -10000 505MB.txt' ran
1.30 ± 0.43 times faster than 'target/release/tail -n -10000 505MB.txt'
5.66 ± 0.71 times faster than 'tail -n -100000 505MB.txt'
6.80 ± 1.07 times faster than 'target/release/tail -n -100000 505MB.txt'
40.35 ± 3.96 times faster than 'target/release/tail -n -1000000 505MB.txt'
41.82 ± 5.63 times faster than 'tail -n -1000000 505MB.txt' The testfile was created with: tr -dc "A-Za-z 0-9" < /dev/urandom | fold -w 100 | head -n 5000000 > 505MB.txt |
@jhscheer I think @tertsdiepraam meant my first benchmark
with To be sure, I reran the above benchmark with the file benchmark with
|
if beginning { |
is false, replacing the ringbuffer, like you suggested?
I think I made a mistake there. The second benchmark made more sense. Anyway it's still an interesting difference.
Please do! |
Rewrite handling of stdin when it is piped and read input in chunks. Fixes uutils#3842
Rewrite handling of stdin when it is piped and read input in chunks. Fixes uutils#3842
Rewrite handling of stdin when it is piped and read input in chunks. Fixes uutils#3842
This fixes a bug where calling `tail - < file.txt` would result in invoking `unbounded_tail()`. However, it is a stdin redirect to a seekable regular file and therefore `bounded_tail` should be invoked as if `tail file.txt` had been called.
tail: fix stdin redirect (#3842)
Rewrite handling of stdin when it is piped and read input in chunks. Fixes #3842
I'm new to rust and I like this project, so I take parts of it sometimes as reference for my rust learning projects. During tests I noticed that uutils tail version is blazingly fast reading large files (my test file
tests/inputs/bigger.txt
is ~500MB full of random text with10_000_000
lines although not very long lines) from disk and is up to 15x faster than the gnu version, but when it comes to reading from stdin the performance drops significantly. Please note, I ran these benchmarks just to get a first impression for relative performance differences between the tested programs. Here's a quick overview over the test filetests/inputs/bigger.txt
, tail and uu-tail:Benchmark of
{tail,uu-tail} -n +{10,1000,100000,10000000} tests/inputs/bigger.txt
in which uu-tail is faster than gnu's tail. However there is a performance drop of uu-tail running with `-n +10_000_000` at the end of the benchmark test run.Now with
-n -{values}
instead of-n +{values}
the gnu version and uu-tail version are pretty close.Benchmark of
{tail,uu-tail} -n -{10,1000,100000,10000000} tests/inputs/bigger.txt
But when it comes to reading from stdin uu-tail is significantly slower than the gnu version
Benchmark of
{tail,uu-tail} -n -{10,1000,100000,10000000} - < tests/inputs/bigger.txt
I hope these benchmarks help figuring out the problem and I haven't done anything wrong on my side.
The text was updated successfully, but these errors were encountered: