Skip to content

head: use std::io::copy() with TakeLines reader - #2712

Merged
sylvestre merged 1 commit into
uutils:masterfrom
jfinkels:head-take-lines-reader
Oct 23, 2021
Merged

head: use std::io::copy() with TakeLines reader#2712
sylvestre merged 1 commit into
uutils:masterfrom
jfinkels:head-take-lines-reader

Conversation

@jfinkels

Copy link
Copy Markdown
Collaborator

Replace the custom split::walk_lines() function with a call to
std::io::copy(), using a new TakeLines reader as the source and
stdout as the destination. The TakeLines reader is an adaptor that
scans the bytes being read for line ending characters and stops the
reading after a given number of lines has been read (similar to the
std::io::Take adaptor).

This change

  • makes the read_n_lines() function more concise,
  • allows it to mirror the implementation of read_n_bytes(),
  • increases the speed of head -n NUM.

Here's my speed benchmark. The file shakespeare.txt contains about 170,000 lines, each less than 100 characters, comprising the collected works of Shakespeare. head is the GNU version, head-master is the version from the current master branch (11ca4be), and head-reader is the version from this branch. We read the first 100,000 lines from the file with each version of head.

$ hyperfine --warmup=100 --runs=500 \
> "head -n100000 shakespeare.txt > /dev/null" \
> "./head-master -n100000 shakespeare.txt > /dev/null" \
> "./head-reader -n100000 shakespeare.txt > /dev/null"
Benchmark #1: head -n100000 shakespeare.txt > /dev/null
  Time (mean ± σ):       5.3 ms ±   2.6 ms    [User: 4.4 ms, System: 1.0 ms]
  Range (min … max):     2.6 ms …  13.3 ms    500 runs
 
  Warning: Command took less than 5 ms to complete. Results might be inaccurate.
 
Benchmark #2: ./head-master -n100000 shakespeare.txt > /dev/null
  Time (mean ± σ):      18.7 ms ±   2.1 ms    [User: 8.2 ms, System: 10.5 ms]
  Range (min … max):    15.1 ms …  24.4 ms    500 runs
 
Benchmark #3: ./head-reader -n100000 shakespeare.txt > /dev/null
  Time (mean ± σ):       5.7 ms ±   3.1 ms    [User: 3.9 ms, System: 1.8 ms]
  Range (min … max):     1.6 ms …  10.4 ms    500 runs
 
  Warning: Command took less than 5 ms to complete. Results might be inaccurate.
 
Summary
  'head -n100000 shakespeare.txt > /dev/null' ran
    1.07 ± 0.78 times faster than './head-reader -n100000 shakespeare.txt > /dev/null'
    3.50 ± 1.76 times faster than './head-master -n100000 shakespeare.txt > /dev/null'

@sylvestre

Copy link
Copy Markdown
Contributor

Impressive gain, could you please document your bench like in src/uu/cut/BENCHMARKING.md ?
thanks

@jfinkels

Copy link
Copy Markdown
Collaborator Author

Yes, I will add a document.

@jfinkels
jfinkels force-pushed the head-take-lines-reader branch from d33eeaa to 524d206 Compare October 14, 2021 00:34
@jfinkels

Copy link
Copy Markdown
Collaborator Author

I have added the BENCHMARKING.md document and rebased on master.

Comment thread src/uu/head/src/head.rs Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AFAIU, this is line buffered whereas GNU head buffers up to 8 kB of output. It might be helpful to wrap stdout within a BufWriter to achieve a similar effect. This could especially beneficial insofar io::copy has a specialisation for BufWriter.

@adamreichold adamreichold Oct 18, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I also noticed that for splitting lines, GNU wc actually uses a larger 16 kB buffer when AVX support is used. Since the memchr crate will transparently use the widest vectors available and supported, it would probably make sense to create the above writer with a 16 kB buffer (which should determine the buffer size passed to the Read implementation).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Okay, I can add that.

@adamreichold

Copy link
Copy Markdown
Contributor

I have added the BENCHMARKING.md document and rebased on master.

Concerning the

Warning: Command took less than 5 ms to complete. Results might be inaccurate.

warnings issued by Hyperfine, the Wikipedia data dumps, e.g. https://dumps.wikimedia.org/wikidatawiki/latest/wikidatawiki-latest-pages-logging.xml.gz, provide text files with on the order of 100M lines which could make for a larger and easier to measure test case?

@jfinkels

Copy link
Copy Markdown
Collaborator Author

[...] the Wikipedia data dumps, e.g. https://dumps.wikimedia.org/wikidatawiki/latest/wikidatawiki-latest-pages-logging.xml.gz, provide text files with on the order of 100M lines which could make for a larger and easier to measure test case?

Okay, I can run that test and add information on how to run it to the BENCHMARKING.md file.

@jfinkels

jfinkels commented Oct 19, 2021

Copy link
Copy Markdown
Collaborator Author

Here is the result on the XML file you suggested:

$ hyperfine --warmup 100 \
> "head -n 1000000 wikidata.xml > /dev/null" \
> "./head-master -n 1000000 wikidata.xml > /dev/null" \
> "./head-branch -n 1000000 wikidata.xml > /dev/null"
Benchmark #1: head -n 1000000 wikidata.xml > /dev/null
  Time (mean ± σ):      18.0 ms ±   0.1 ms    [User: 13.4 ms, System: 4.6 ms]
  Range (min … max):    17.8 ms …  18.2 ms    162 runs
 
Benchmark #2: ./head-master -n 1000000 wikidata.xml > /dev/null
  Time (mean ± σ):     146.4 ms ±   1.1 ms    [User: 65.6 ms, System: 80.7 ms]
  Range (min … max):   144.7 ms … 148.0 ms    20 runs
 
Benchmark #3: ./head-branch -n 1000000 wikidata.xml > /dev/null
  Time (mean ± σ):      13.0 ms ±   0.8 ms    [User: 8.8 ms, System: 4.3 ms]
  Range (min … max):    12.0 ms …  14.9 ms    228 runs
 
Summary
  './head-branch -n 1000000 wikidata.xml > /dev/null' ran
    1.38 ± 0.08 times faster than 'head -n 1000000 wikidata.xml > /dev/null'
   11.24 ± 0.69 times faster than './head-master -n 1000000 wikidata.xml > /dev/null'

I'll add BufWriter and see if that changes anything.

Edit: The XML file contains about 130 million lines, and in this test I am reading the first one million lines.

@jfinkels

Copy link
Copy Markdown
Collaborator Author

Here's what I saw after wrapping the stdout in a BufWriter:

Benchmark #1: head -n 1000000 wikidata.xml > /dev/null
  Time (mean ± σ):      17.3 ms ±   0.1 ms    [User: 13.4 ms, System: 4.1 ms]
  Range (min … max):    17.1 ms …  17.6 ms    162 runs
 
Benchmark #2: ./head-master -n 1000000 wikidata.xml > /dev/null
  Time (mean ± σ):     144.8 ms ±   0.9 ms    [User: 65.2 ms, System: 79.7 ms]
  Range (min … max):   143.7 ms … 147.2 ms    20 runs
 
Benchmark #3: ./head-branch -n 1000000 wikidata.xml > /dev/null
  Time (mean ± σ):      11.2 ms ±   0.1 ms    [User: 7.3 ms, System: 4.2 ms]
  Range (min … max):    11.1 ms …  11.7 ms    239 runs
 
Summary
  './head-branch -n 1000000 wikidata.xml > /dev/null' ran
    1.54 ± 0.02 times faster than 'head -n 1000000 wikidata.xml > /dev/null'
   12.89 ± 0.14 times faster than './head-master -n 1000000 wikidata.xml > /dev/null'

@adamreichold

Copy link
Copy Markdown
Contributor

Thank you for going the extra mile! I find it notable that the difference to GNU head is mostly in user time, not in system time. I suspect this is due to GNU head not using SIMD to detect newlines AFAICS.

Replace the custom `split::walk_lines()` function with a call to
`std::io::copy()`, using a new `TakeLines` reader as the source and
`stdout` as the destination. The `TakeLines` reader is an adaptor that
scans the bytes being read for line ending characters and stops the
reading after a given number of lines has been read (similar to the
`std::io::Take` adaptor).

This change

* makes the `read_n_lines()` function more concise,
* allows it to mirror the implementation of `read_n_bytes()`,
* increases the speed of `head -n NUM`.
@jfinkels
jfinkels force-pushed the head-take-lines-reader branch from 76ca123 to 858b0a9 Compare October 21, 2021 00:59
@jfinkels

Copy link
Copy Markdown
Collaborator Author

I rebased and squashed the commits

@sylvestre
sylvestre merged commit 811698b into uutils:master Oct 23, 2021
@jfinkels
jfinkels deleted the head-take-lines-reader branch December 30, 2021 23:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants