Skip to content
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

perf(parser): Use more performant operations for line parsing #198

Merged
merged 3 commits into from Mar 14, 2019

Conversation

Marwes
Copy link
Collaborator

@Marwes Marwes commented Mar 13, 2019

take_until_bytes uses memchr to find the next end of line + some
minor changes to use the fact that we know that line contains \r\n
at the end.

decode                  time:   [2.1868 us 2.2129 us 2.2494 us]
                        change: [-30.297% -29.538% -28.706%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) high mild
  1 (1.00%) high severe

Markus Westerlind added 2 commits March 12, 2019 12:15
`take_until_bytes` uses `memchr` to find the next end of line + some
minor changes to use the fact that we know that `line` contains `\r\n`
at the end.

```
decode                  time:   [2.1868 us 2.2129 us 2.2494 us]
                        change: [-30.297% -29.538% -28.706%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) high mild
  1 (1.00%) high severe
```
@badboy
Copy link
Collaborator

badboy commented Mar 13, 2019

First off: really appreciate all this perf work here! Awesome!

This is causing issues at the moment though:

thread 'main' panicked at 'Could not read enough bytes', tests/parser.rs:102:9
thread 'main' panicked at '[quickcheck] TEST FAILED (runtime error). Arguments: (ArbitraryValue(bulk(string-data('""'))), PartialWithErrors { items: [Limited(7)], _marker: PhantomData })
Error: "Could not read enough bytes"', /home/travis/.cargo/registry/src/github.com-1ecc6299db9ec823/quickcheck-0.6.2/src/tester.rs:179:28

@Marwes
Copy link
Collaborator Author

Marwes commented Mar 13, 2019

Thought I didn't need to add a partial parse test for take_until_bytes specifically since I just generalized an existing parser to implement it. Seems that one lacked a test or two though. Good thing it was caught here at least!

@@ -26,7 +26,7 @@ sha1 = ">= 0.2, < 0.7"
url = "1.2"
rustc-serialize = { version = "0.3.16", optional = true }
unix_socket = { version = "0.5.0", optional = true }
combine = "3.8.0"
combine = "3.8.1"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wait, what? checks combine commits

src/parser.rs Show resolved Hide resolved
@badboy badboy merged commit 859c453 into redis-rs:master Mar 14, 2019
@Marwes Marwes deleted the bench_decode branch March 14, 2019 21:15
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.

None yet

2 participants