Skip to content

Implement wc fast paths that skip Unicode decoding.#3740

Merged
sylvestre merged 2 commits intouutils:mainfrom
resistor:main
Jul 25, 2022
Merged

Implement wc fast paths that skip Unicode decoding.#3740
sylvestre merged 2 commits intouutils:mainfrom
resistor:main

Conversation

@resistor
Copy link
Contributor

Byte, character, and line counting can all be done on the raw bytes
of the incoming stream without decoding the Unicode characters. This
fact was previously exploited in specific fast paths for counting
characters and counting lines. This change unifies those fast paths into
a single shared fast paths, using const generics to specialize the
function for each use case. This has the benefit of making sure that all
combinations of these Unicode-oblivious fast paths benefit from the same
optimization.

On my laptop, this speeds up wc -clm odyssey1024.txt from 840ms to
120ms. I experimented with using a filter loop for line counting, but
continuing to use the bytecount crate came out ahead by a significant
margin.

@anastygnome
Copy link
Contributor

@resistor good job, but needs a cargo fmt run ;)

Byte, character, and line counting can all be done on the raw bytes
of the incoming stream without decoding the Unicode characters. This
fact was previously exploited in specific fast paths for counting
characters and counting lines. This change unifies those fast paths into
a single shared fast paths, using const generics to specialize the
function for each use case. This has the benefit of making sure that all
combinations of these Unicode-oblivious fast paths benefit from the same
optimization.

On my laptop, this speeds up `wc -clm odyssey1024.txt` from 840ms to
120ms. I experimented with using a filter loop for line counting, but
continuing to use the bytecount crate came out ahead by a significant
margin.
@resistor
Copy link
Contributor Author

@resistor good job, but needs a cargo fmt run ;)

Done

@sylvestre
Copy link
Contributor

the coverage indicates that we are missing some tests, could you please add them ?
thanks

@resistor
Copy link
Contributor Author

the coverage indicates that we are missing some tests, could you please add them ?
thanks

Done

@anastygnome
Copy link
Contributor

@sylvestre this is now good to merge (tail-related failures) ;)
Congrats @resistor !

@sylvestre
Copy link
Contributor

indeed, thanks :)

@sylvestre sylvestre merged commit 2fa4d6a into uutils:main Jul 25, 2022
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